This is the mail archive of the
mailing list for the eCos project.
Re: MMAPPED disk package
- From: "Anthony Tonizzo" <atonizzo at gmail dot com>
- To: "Jonathan Larmour" <jifl at ecoscentric dot com>
- Cc: "Andrew Lunn" <andrew at lunn dot ch>, ecos-patches at sourceware dot org
- Date: Tue, 21 Nov 2006 11:29:09 -0800
- Subject: Re: MMAPPED disk package
- Domainkey-signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=tXJWIBlq1bqnLYtgcztksRhnIwRuRNzjW43K25ekNTQj+PFwSCU8vC0T7gUCH69A+/A3ptG7FiJIzV5744S4qaiFssVwo2EGaC1WLj9e1P/IVjnZwpNlE/U8s84tLzodWnSIAJupb5XcqtMPRHCBB3G8ln2pwrj8EvkzCxvjp/I=
- References: <firstname.lastname@example.org> <20061120234538.GE30460@lunn.ch> <45633EDB.5090709@eCosCentric.com>
I was assuming that's because it started from the IDE driver, by Savin. But
I guess that needs checking. So the only thing that matters is if Savin
added new code, other than what was already in CVS.
I did not even look at the IDE driver although in retrospect I should have.
The driver I started from was actually the /devs/disk/v85x which, with minor
changes, could be superseded by my package now. That package
implements the memory mapped interface to ATA. IDE is just another
one of the ATA interfaces.
I would normally put things like this in files called
var_io.h/proc_io.h/plf_io.h. In this case, I see mpc8xx already has a
var_regs.h file, but this new file is a bit namespace polluting, and so it
might be better to keep it separate after all. Plus it was pulled in from
elsewhere as a standalone file anyway. I suggest mpc8xx_regs.h.
These are masks specific to the mpc8xx, so the name seems alright
I think these defines need to be given CYG_ prefixes, to control the
#define DISK_HW_INIT(a) PCMCIAInit((cyg_int32)a);
#define HW_DISK_MEMORY_BASE 0xFA000000
Prefixing each with CYG_DEVS_DISKS_CF_ would be better.
I forgot to modify the CF_DISK_INSTANCE macro and strip any CF reference
from it. I am going to do it like its done in the IDE package. While a CardFlash
was indeed used for its testing, I strived to be as
general as possible in this implementation. As soon as I find a moment I
would like to connect a hard drive to the MBX and use this package
instead of the IDE one to access it. As far as I am concerned, the CVS
tree should really be changed into something like:
etc. since both my package and the IDE package are equipotent
implementations of part of the ATA spec. The package I submitted
can be used with CardFlash only because CardFlash memory
comes equipped with an ATA controller on board to make it look
like a hard drive.
Likewise other defines in plf_io.h.
Minor issue, but mmapped_disk.h has this at the top:
CYGONCE_HAL_DEV_DISK_MMAPPED_H but isn't a HAL file. Similarly in the CDL
CYGHWR_HAL_DEVS_DISKS_MMAPPED_IWIDTH wants "_HAL" removed.
More important is that that file says "Originally a file from
Motorola/Frescale." in which case we need to confirm the license and
redistribution terms of that file.
The original file I used came with no copyright assignment however if
you try to download any example code for the MPC860 from the Frescale
you download the code. I guess that is what you are looking for.
The initialisation doesn't seem quite right. It seems to me that the
CF_DISK_INSTANCE macro should be defined in an exported header file, and
then the relevant platform HAL should instantiate devices using that macro.
That way it can choose the appropriate number of instances for the
hardware. The CYGVAR_DEVS_DISK_MMAPED_DISK0 CDL would thus be removed, and
dev_disk_mmapped.c no longer forced into libextras.a (instead the above
instantiating HAL file would be instead).
I will go back and look at this. This is one of the things that was not
modified from the existing v85x package from Savin.
This also gives the possibility, in the HAL, of disabling a CF interface by
CDL. This could be important, e.g. on powerpc MBX, so that you can add
CYGPKG_IO_DISK to your config, but not get potentially unnecessary CF
support. You might be wanting support for SPI dataflash, not CF so there
needs to be a way to disable it.
I am not sure I understand how this would work. I will think about it.
I think some of them the 16-bit versus 8-bit routines could be merged and
thus simplified, and instead the choice being done by an #ifdef around a
typedef, and a one-off #ifdef in the function body to choose
CF_REG16_DUPLICATE_EVEN_DATA versus CF_REG8_DATA_8BIT.
There's several magic numbers in cf_disk_init that deserve to be made
#defines (I'm noticing 0x200/0x202/0x204/0x848A at least).