Hi Alexander, On Fri, Jun 19, 2015 at 06:46:02PM +0300, Alexander Smirnov wrote: > decode-dimms perl script is used as prototype > (see https://github.com/groeck/i2c-tools/blob/master/eeprom/decode-dimms). > > Here is a sample decode output: > > barebox@barebox sandbox:/ decode env/crucial_pc2-6400_ddr2 > Decoding EEPROM: env/crucial_pc2-6400_ddr2 > > ---=== SPD EEPROM Information ===--- > EEPROM Checksum of bytes 0-62 OK (0xCA) > Total number of bytes in EEPROM 256 > Fundamental Memory type DDR2 SDRAM > SPD Revision 1.3 > > ---=== Memory Characteristics ===--- > Maximum module speed 800 MHz (PC2-6400) > Size 1024 MB > Banks x Rows x Columns x Bits 8 x 14 x 10 x 64 > Ranks 1 > SDRAM Device Width 8 bits > Module Height 30.0 mm > Module Type SO-DIMM (67.6 mm) > DRAM Package Planar > Voltage Interface Level SSTL 1.8V > Module Configuration Type No Parity > Refresh Rate Reduced (7.8 us) - Self Refresh > Supported Burst Lengths 4, 8 > Supported CAS Latencies (tCL) 6T > tCL-tRCD-tRP-tRAS 6-6-6-18 as DDR2-800 > Minimum Cycle Time 250 (ns*100) at CAS 6 > Maximum Access Time 4 (ns*10) at CAS 6 > Maximum Cycle Time (tCK max) 8 ns > > ---=== Timing Parameters ===--- > Address/Command Setup Time Before Clock (tIS) 17 (ns*100) Better print these in ns directly? > +static int do_decode(int argc, char *argv[]) > +{ > + int highestCAS = 0; > + int cas[256]; > + int i, i_i, k; > + int ddrclk, tbits, pcclk; > + int trcd, trp, tras; > + int ctime; > + int fp; > + uint8_t record[256]; > + uint8_t parity; > + char *ref; > + > + if (argc != 2) { > + printf("Отсутствует или указано больше 1 аргумента\n"); I think most readers can't read this language ;) > + return 2; > + } > + > + fp = open(argv[1], O_RDONLY); better 'fd'? It's a file descriptor, not a file pointer. > + > + if (fp == 0) { > + perror("Ошибка при работе с файлом"); > + return 3; > + } > + > + read(fp, &record[0], 256); > + close(fp); You should separate the command code from the dispatching/printing code. let the dispatching/printing code take a void *buf and put it into common/. This way we can call this not only from the command but also from C code which makes this more useful. > + > +// dump(&record[0], 256); > + > + printf("Decoding EEPROM: %s\n\n", argv[1]); > + > + printf("---=== SPD EEPROM Information ===---\n"); > + printf("EEPROM Checksum of bytes 0-62\t\t\t OK (0x%0X)\n", record[63]); > + printf("# of bytes written to SDRAM EEPROM\t\t %d\n", record[0]); > + printf("Total number of bytes in EEPROM\t\t\t %d\n", 1 << record[1]); > + > + if (record[2] < 11) { > + printf("Fundamental Memory type\t\t\t\t %s\n", type_list[record[2]]); > + } else { > + printf("Warning: unknown memory type (%02x)\n", record[2]); > + } Please skip the braces when they are not needed. Generally we already have include/ddr_spd.h. If you put this struct over the buffer then the dispatching should be more straight forward. Wouldn't that be possible? Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox