public inbox for ecos-patches@sourceware.org
 help / color / mirror / Atom feed
From: Bart Veer <bartv@ecoscentric.com>
To: Chris Holgate <chris@zynaptic.com>
Cc: ecos-patches@sourceware.org
Subject: Re: tidy STM32 SPI driver
Date: Wed, 11 Feb 2009 14:33:00 -0000	[thread overview]
Message-ID: <pnbpt9dovy.fsf@delenn.bartv.net> (raw)
In-Reply-To: <4992B692.2060002@zynaptic.com> (message from Chris Holgate on 	Wed, 11 Feb 2009 11:29:22 +0000)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3943 bytes --]

>>>>> "Chris" == Chris Holgate <chris@zynaptic.com> writes:

    >> This SPI driver did not use linker garbage collection the way I
    >> intended for SPI drivers. It created a C++ object in
    >> src/spi_stm32_init.cxx which was places in libextras.a. Hence
    >> the C++ object was always part of any application link, and
    >> because of the KEEP(*.ctors) in the linker script its
    >> constructor would never get garbage-collected. Net result: all
    >> applications would end up with the C++ object, the constructor,
    >> and the cyg_spi_cortexm_stm32_init() function, even if SPI was
    >> never used.

    Chris> That bit of code was essentially a cut and paste from the
    Chris> AT91 SPI driver, which sounds as though it may suffer from
    Chris> the same problem.

It does - and is next on my list of things to fix.

    >> I also looked at moving the three SPI buses into separate
    >> modules, built unconditionally. Again linker magic would ensure
    >> that only buses actually used by the application would end up
    >> in the application, with no need for developers to enable the
    >> relevant CDL components. This does not look entirely feasible
    >> right now, there are too many per-bus configuration options.

    Chris> Getting rid of the 'bus enable' checkboxes would definitely
    Chris> be a good thing.

    Chris> Generally, the configuration options are only used to
    Chris> statically initialise the 3 bus data structures - so it
    Chris> should be fairly easy to pull each bus data structure
    Chris> declaration into a separate compile unit with per-bus
    Chris> constructors and function tables. The common core functions
    Chris> would then only be pulled in if at least one of the per-bus
    Chris> compile units were actually referenced.

That is the general idea. Ideally things always "just work", with some
careful coding and linker garbage collection eliminating any
functionality that the application does not actually need.
Configuration options can be used when that is not possible, but do
require more effort from application developers - especially in the
absence of any driver documentation.

For the chip select handling, if I understand the code correctly then
currently the SPI driver is responsible for both setting up the GPIO
pins and for driving them. There are other ways of handling this. One
approach is for the platform HAL to set up all the pins early on
during initialization, before the static constructors, on the theory
that the platform HAL author knows what devices are attached to which
pins. That saves having to do pin setup in each driver, so saves a bit
more code.

A more flexible approach is used in the coldfire HALs, see the mcf5272
processor HAL's gpio.cdl and CYGHWR_HAL_M68K_MCF5272_BOARD_PINS in the
m5272c3 platform HAL. That exposes all the pin settings to the
configuration system, with the platform HAL providing default
settings. The approach makes it possible to support boards with
expansion connectors etc., where the platform HAL author does not
necessarily know exactly what hardware will be used. It offers a lot
of flexibility, and involves very little target-side code or data, but
at the cost of significant programming effort.

If pin setup can be left to the platform HAL then the SPI driver only
needs to worry about driving the chip select pins. That can be
handled by putting the pin id into the SPI device structure instead of
the bus structure. That should save more code and data.

Bart

-- 
Bart Veer                                   eCos Configuration Architect
eCosCentric Limited    The eCos experts      http://www.ecoscentric.com/
Barnwell House, Barnwell Drive, Cambridge, UK.      Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
Besuchen Sie uns vom 3.-5.03.09 auf der Embedded World 2009, Stand 11-300
Visit us at Embedded World 2009, Nürnberg, Germany, 3-5 Mar, Stand 11-300

  reply	other threads:[~2009-02-11 14:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-11 11:29 Chris Holgate
2009-02-11 14:33 ` Bart Veer [this message]
2009-02-11 15:23   ` Nick Garnett
2009-02-11 15:37   ` Chris Holgate
  -- strict thread matches above, loose matches on Subject: below --
2009-02-10 22:31 Bart Veer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pnbpt9dovy.fsf@delenn.bartv.net \
    --to=bartv@ecoscentric.com \
    --cc=chris@zynaptic.com \
    --cc=ecos-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).