Re: Motorola Coldfire MCF52xx I2C device driver

[Bart Veer <bartv at ecoscentric dot com>]

>>>>> "Andrew" == Andrew Lunn <andrew@lunn.ch> writes:

    Andrew> Hi Bart
    Andrew> On Tue, Oct 25, 2005 at 08:58:47AM +0200, uwe.kindler@cetoni.de wrote:
    >> Hello Andrew,
    >> 
    >> here is a new patch that contains all your changes.
    >> 
    >> About using is_loaded() in the CDL option CYGNUM_DEVS_I2C_MCF52xx_ISR_PRIORITY_MODULE:
    >> 
    >> I work with the eCos Pro starterkit for SSV DilNet PC. The ethernet
    >> driver supplied with this starter kit also uses is_loaded() for
    >> checking if the platform HAL provides a valid default interrupt
    >> priority.
    >> 
    >> Here is the part of the platform CDL file that defines the interrupt priority:
    >> 
    >> cdl_option CYGNUM_HAL_M68K_MCF52xx_ISR_DEFAULT_PRIORITY_I2C {
    >> display "Default priority for I2C interrupts"
    >> flavor data
    >> calculated 45
    >> description "
    >> This configuration value provides a suitable default value for
    >> the I2C interrupt priority."
    >> }
    >> 
    >> So I thought it would be a good idea to do it the same way like the supplied drivers in eCos Pro starterkit.

    Andrew> Bart, the bit of CDL in question is:

    Andrew>             cdl_option CYGNUM_DEVS_I2C_MCF52xx_ISR_PRIORITY_MODULE[set ::i2c_mcf52xx] {
    Andrew>                 display     "Interrupt priority"
    Andrew>                 flavor      data
    Andrew>                 default_value   is_loaded(CYGNUM_HAL_M68K_MCF52xx_ISR_DEFAULT_PRIORITY_I2C) ? \
    Andrew>                                 CYGNUM_HAL_M68K_MCF52xx_ISR_DEFAULT_PRIORITY_I2C : \
    Andrew>                                 CYGNUM_HAL_M68K_MCF52xx_ISR_PRIORITY_MIN
    Andrew>                 legal_values   CYGNUM_HAL_M68K_MCF52xx_ISR_PRIORITY_MIN to CYGNUM_HAL_M68K_MCF52xx_ISR_PRIORITY_MAX
    Andrew>                 description "
    Andrew>                     Interrupt priority for this I2C module."
    Andrew>             }

    Andrew> The question is about the use of is_loaded(). is_loaded()
    Andrew> will return true when the option exists. The option may be
    Andrew> inactive, but is_loaded will still return true. For me,
    Andrew> using is_active() is more logic.

Strictly speaking you are correct, is_active() is more accurate than
is_loaded(). In practice it should not matter.

Some background: the mcf5282's interrupt controller has some unusual
features. On most processors interrupt priorities can start at 0, but
on the mcf5282 valid interrupt priorities are in the range 8 to 55.
Each enabled device must have a unique interrupt priority. If you
accidentally give two devices the same priority and they interrupt at
the same time then very strange things happen. Hence the mcf5282
processor HAL defines default priorities for the on-chip peripherals,
e.g. CYGNUM_HAL_M68K_MCF52xx_ISR_DEFAULT_PRIORITY_I2C. Usually the
device drivers will stick with those defaults and everything just
works, but it is possible for application developers to fine-tune the
priorities if desired.

The processor HAL defines
CYGNUM_HAL_M68K_MCF52xx_ISR_DEFAULT_PRIORITY_I2C as a calculated
option directly below CYGPKG_HAL_M68K_MCF5282, so it is always active
in any configuration involving an mcf5282 processor. Default
priorities are fixed, just like endianness or the sizes of various
data types. I cannot foresee any scenario where I would want the
default priorities to be loaded but inactive, so is_loaded() and
is_active() achieve the same effect.

Other coldfire processors may have the same I2C device but a different
interrupt controller without the restrictions. The processor HALs for
those would not define default interrupt priorities so the device
driver will just default to CYGNUM_HAL_M68K_MCF52xx_ISR_PRIORITY_MIN.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts