This is the mail archive of the ecos-devel@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Serial driver for ARM s3c4510


Andrew Lunn wrote:
Additionally the AIM platform includes a 16550 UART, so this serial interface
should be implemented as /dev/ser2 also.
Is it possible and useful to make a generic s3c4510 serial driver, use the
generic 16x5x driver but make only one package (devs/serial/arm/aim) which includes this two drivers. Or is it better to make two packages like devs/serial/arm/aim_s3c4510 and devs/serial/arm/aim_16x5x?


I would go for two packages. There is no logical connection between
the two, so why should they share a package? What happens if in the
future you replace your 16x5x with something else etc?


You are the second person who recently needed two different serial
drivers in the same configuration. I don't really know the serial
driver architecture that well, but from when i looked last the
architecture is not very good at this. If you have time, it would be
good it you could study the architecture and see if you can make is
better for supporting multiple devices.

I think for that reason it may be best to include all platform specific elements of a serial driver in a single package. But even then, it's possible there might be hiccups.


The most obvious issue is this in e.g. the 16x5x driver:
puts $::cdl_system_header "#ifndef CYGDAT_IO_SERIAL_DEVICE_HEADER"
puts $::cdl_system_header "#define CYGDAT_IO_SERIAL_DEVICE_HEADER <pkgconf/io_serial_generic_16x5x.h>"


However I'm not sure what info actually _must_ be made public like this, other than e.g.:
cdl_option CYGPRI_SER_TEST_SER_DEV {
display "Serial device used for testing"
flavor data
default_value { CYGDAT_IO_SERIAL_I386_PC_SERIAL0_NAME }
}


define_proc {
puts $::cdl_header "#define CYGPRI_SER_TEST_CRASH_ID \"i386pc\""
puts $::cdl_header "#define CYGPRI_SER_TEST_TTY_DEV \"/dev/tty0\""
}


There would be two ways to fix this probably.... change those #defines to $::cdl_system_header instead, so that the generic serial layer can get them that way; or create a CYGPRI_SER_TEST_CRASH_ID and CYGPRI_SER_TEST_TTY_DEV option in the generic serial driver (or perhaps a subtle rename to avoid clashes with existing clashes) and use CDL requires statements in the child packages to set values. The latter would be cleaner as it doesn't involve touching system.h which could cause unnecessary rebuilds.

It's a shame that CDL has this restriction with the "define" CDL property: "The -file option can be used to specify an alternative destination. At the time of writing the only valid alternative definition is -file=system.h, which will send the output to the global configuration header file pkgconf/system.h." I'll create a bugzilla bug for this.

Jifl
--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
--["No sense being pessimistic, it wouldn't work anyway"]-- Opinions==mine


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]