This is the mail archive of the 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: MMAPPED disk package

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 with me.

I think these defines need to be given CYG_ prefixes, to control the
namespace better:
cyg_bool PCMCIAInit(cyg_int32);
#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

Will correct.

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 site there is a EULA page whose terms of use you have to accept before 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

There's several magic numbers in cf_disk_init that deserve to be made
#defines (I'm noticing 0x200/0x202/0x204/0x848A at least).

Will change.


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