From: Andriy G. <av...@ic...> - 2009-09-07 16:28:26
|
Latest FreeBSD has an option of using ahci(4) driver which provides access to SATA disks via CAM. In that case the disk devices are named ada*, they do not support ATA ioctls and ATA commands should be sent via cam(3). Here is the first take at implementing support for ada devices. Auto-discovery of ada devices is not implemented yet. It should be fairly easy to do using camcontrol.c code as an example. Code in freebsd_atacam_device::do_cmd should really use more of atacam helper functions. I am not sure if my approach of making freebsd_atacam_device a subclass of freebsd_ata_device is such a good idea, but I took it for a fast re-use of ata_command_interface method. Index: os_freebsd.cpp =================================================================== --- os_freebsd.cpp (revision 2902) +++ os_freebsd.cpp (working copy) @@ -64,6 +72,7 @@ #define CONTROLLER_3WARE_678K_CHAR 0x06 // set by guess_device_type() #define CONTROLLER_HPT 0x09 // SATA drives behind HighPoint Raid controllers #define CONTROLLER_CCISS 0x10 // CCISS controller +#define CONTROLLER_ATACAM 0x04 static __unused const char *filenameandversion="$Id$"; @@ -302,6 +311,7 @@ // osst, nosst and sg. static const char * fbsd_dev_prefix = _PATH_DEV; static const char * fbsd_dev_ata_disk_prefix = "ad"; +static const char * fbsd_dev_atacam_disk_prefix = "ada"; static const char * fbsd_dev_scsi_disk_plus = "da"; static const char * fbsd_dev_scsi_pass = "pass"; static const char * fbsd_dev_scsi_tape1 = "sa"; @@ -319,6 +329,7 @@ // No Autodetection if device type was specified by user if (*type){ if(!strcmp(type,"ata")) return CONTROLLER_ATA; + if(!strcmp(type,"atacam")) return CONTROLLER_ATACAM; if(!strcmp(type,"cciss")) return CONTROLLER_CCISS; if(!strcmp(type,"scsi") || !strcmp(type,"sat")) goto handlescsi; if(!strcmp(type,"3ware")){ @@ -341,6 +352,13 @@ // else advance pointer to following characters dev_name += dev_prefix_len; } + + // form /dev/ada* or ada* + if (!strncmp(fbsd_dev_atacam_disk_prefix, dev_name, + strlen(fbsd_dev_atacam_disk_prefix))) { + return CONTROLLER_ATACAM; + } + // form /dev/ad* or ad* if (!strncmp(fbsd_dev_ata_disk_prefix, dev_name, strlen(fbsd_dev_ata_disk_prefix))) { @@ -475,6 +493,14 @@ return false; } } + if (parse_ok == CONTROLLER_ATACAM) { + if ((fdchan->camdev = ::cam_open_device(dev,O_RDWR)) == NULL) { + perror("cam_open_device"); + free(fdchan); + errno = ENOENT; + return false; + } + } if (parse_ok == CONTROLLER_3WARE_678K_CHAR) { char buf[512]; @@ -562,7 +588,10 @@ if (fdchan->atacommand) failed=::close(fdchan->atacommand); #endif - + + if (fdchan->camdev != NULL) + cam_close_device(fdchan->camdev); + // if close succeeded, then remove from device list // Eduard, should we also remove it from list if close() fails? I'm // not sure. Here I only remove it from list if close() worked. @@ -590,6 +619,10 @@ protected: virtual int ata_command_interface(smart_command_set command, int select, char * data); + + #ifdef IOCATAREQUEST + virtual int do_cmd(struct freebsd_dev_channel* con, struct ata_ioc_request* request); + #endif }; freebsd_ata_device::freebsd_ata_device(smart_interface * intf, const char * dev_name, const char * req_type) @@ -598,6 +631,72 @@ { } +int freebsd_ata_device::do_cmd(struct freebsd_dev_channel* con, struct ata_ioc_request* request) +{ + return ioctl(con->device, IOCATAREQUEST, request); +} + +#if __FreeBSD_version > 800100 +class freebsd_atacam_device : public freebsd_ata_device +{ +public: + freebsd_atacam_device(smart_interface * intf, const char * dev_name, const char * req_type) + : smart_device(intf, dev_name, "atacam", req_type), freebsd_ata_device(intf, dev_name, req_type) + {} + +protected: + virtual int do_cmd(struct freebsd_dev_channel* con, struct ata_ioc_request* request); +}; + +int freebsd_atacam_device::do_cmd(struct freebsd_dev_channel* con, struct ata_ioc_request* request) +{ + union ccb ccb; + int camflags; + + memset(&ccb, 0, sizeof(ccb)); + + if (request->count == 0) + camflags = CAM_DIR_NONE; + else if (request->flags == ATA_CMD_READ) + camflags = CAM_DIR_IN; + else + camflags = CAM_DIR_OUT; + + cam_fill_ataio(&ccb.ataio, + 0, + NULL, + camflags, + MSG_SIMPLE_Q_TAG, + (u_int8_t*)request->data, + request->count, + request->timeout); + + // ata_28bit_cmd + ccb.ataio.cmd.flags = 0; + ccb.ataio.cmd.command = request->u.ata.command; + ccb.ataio.cmd.features = request->u.ata.feature; + ccb.ataio.cmd.lba_low = request->u.ata.lba; + ccb.ataio.cmd.lba_mid = request->u.ata.lba >> 8; + ccb.ataio.cmd.lba_high = request->u.ata.lba >> 16; + ccb.ataio.cmd.device = 0x40 | ((request->u.ata.lba >> 24) & 0x0f); + ccb.ataio.cmd.sector_count = request->u.ata.count; + + ccb.ccb_h.flags |= CAM_DEV_QFRZDIS; + + if (cam_send_ccb(con->camdev, &ccb) < 0) { + err(1, "cam_send_ccb"); + return -1; + } + + if ((ccb.ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) + return 0; + + cam_error_print(con->camdev, &ccb, CAM_ESF_ALL, CAM_EPF_ALL, stderr); + return -1; +} + +#endif + int freebsd_ata_device::ata_command_interface(smart_command_set command, int select, char * data) { int fd=get_fd(); @@ -741,8 +840,8 @@ unsigned const char failed_lo=0xf4, failed_hi=0x2c; unsigned char low,high; - #ifdef IOCATAREQUEST - if ((retval=ioctl(con->device, IOCATAREQUEST, &request)) || request.error) + #ifdef IOCATAREQUEST + if ((retval=do_cmd(con, &request)) || request.error) #else if ((retval=ioctl(con->atacommand, IOCATA, &iocmd)) || request.error) #endif @@ -777,8 +876,8 @@ return 0; } - #ifdef IOCATAREQUEST - if ((retval=ioctl(con->device, IOCATAREQUEST, &request)) || request.error) + #ifdef IOCATAREQUEST + if ((retval=do_cmd(con, &request)) || request.error) #else if ((retval=ioctl(con->atacommand, IOCATA, &iocmd)) || request.error) #endif @@ -1571,6 +1670,8 @@ protected: virtual ata_device * get_ata_device(const char * name, const char * type); + virtual ata_device * get_atacam_device(const char * name, const char * type); + virtual scsi_device * get_scsi_device(const char * name, const char * type); virtual smart_device * autodetect_smart_device(const char * name); @@ -1604,6 +1705,11 @@ return new freebsd_ata_device(this, name, type); } +ata_device * freebsd_smart_interface::get_atacam_device(const char * name, const char * type) +{ + return new freebsd_atacam_device(this, name, type); +} + scsi_device * freebsd_smart_interface::get_scsi_device(const char * name, const char * type) { return new freebsd_scsi_device(this, name, type); @@ -2162,6 +2268,8 @@ switch (guess) { case CONTROLLER_ATA : return new freebsd_ata_device(this, name, ""); + case CONTROLLER_ATACAM : + return new freebsd_atacam_device(this, name, ""); case CONTROLLER_SCSI: // Try to detect possible USB->(S)ATA bridge if (get_usb_id(name, vendor_id, product_id, version)) { Index: os_freebsd.h =================================================================== --- os_freebsd.h (revision 2902) +++ os_freebsd.h (working copy) @@ -92,6 +92,7 @@ #endif char* devname; // the SCSI device name int unitnum; // the SCSI unit number + struct cam_device *camdev; }; #define FREEBSD_MAXDEV 64 -- Andriy Gapon |
From: Alex S. <ml...@os...> - 2009-09-07 16:52:07
|
Andriy Gapon wrote: > Latest FreeBSD has an option of using ahci(4) driver which provides access to > SATA disks via CAM. In that case the disk devices are named ada*, they do not > support ATA ioctls and ATA commands should be sent via cam(3). > > Here is the first take at implementing support for ada devices. > Auto-discovery of ada devices is not implemented yet. It should be fairly easy > to do using camcontrol.c code as an example. > Code in freebsd_atacam_device::do_cmd should really use more of atacam helper > functions. > > I am not sure if my approach of making freebsd_atacam_device a subclass of > freebsd_ata_device is such a good idea, but I took it for a fast re-use of > ata_command_interface method. > > Hello Andriy, Thank you for your patches. What do you think - may be its better to handle such devices (initially) as scsi, since we already having code for cam_open, cam_close, etc? Auto discovery already implemented for cam devices, so it should not be a problem to extend existing code to support ada* devices. If you want me to commit this patches after review - please, open the ticket and attach patch to it. > Index: os_freebsd.cpp > =================================================================== > --- os_freebsd.cpp (revision 2902) > +++ os_freebsd.cpp (working copy) > @@ -64,6 +72,7 @@ > #define CONTROLLER_3WARE_678K_CHAR 0x06 // set by guess_device_type() > #define CONTROLLER_HPT 0x09 // SATA drives behind HighPoint > Raid controllers > #define CONTROLLER_CCISS 0x10 // CCISS controller > +#define CONTROLLER_ATACAM 0x04 > > static __unused const char *filenameandversion="$Id$"; > > @@ -302,6 +311,7 @@ > // osst, nosst and sg. > static const char * fbsd_dev_prefix = _PATH_DEV; > static const char * fbsd_dev_ata_disk_prefix = "ad"; > +static const char * fbsd_dev_atacam_disk_prefix = "ada"; > static const char * fbsd_dev_scsi_disk_plus = "da"; > static const char * fbsd_dev_scsi_pass = "pass"; > static const char * fbsd_dev_scsi_tape1 = "sa"; > @@ -319,6 +329,7 @@ > // No Autodetection if device type was specified by user > if (*type){ > if(!strcmp(type,"ata")) return CONTROLLER_ATA; > + if(!strcmp(type,"atacam")) return CONTROLLER_ATACAM; > if(!strcmp(type,"cciss")) return CONTROLLER_CCISS; > if(!strcmp(type,"scsi") || !strcmp(type,"sat")) goto handlescsi; > if(!strcmp(type,"3ware")){ > @@ -341,6 +352,13 @@ > // else advance pointer to following characters > dev_name += dev_prefix_len; > } > + > + // form /dev/ada* or ada* > + if (!strncmp(fbsd_dev_atacam_disk_prefix, dev_name, > + strlen(fbsd_dev_atacam_disk_prefix))) { > + return CONTROLLER_ATACAM; > + } > + > // form /dev/ad* or ad* > if (!strncmp(fbsd_dev_ata_disk_prefix, dev_name, > strlen(fbsd_dev_ata_disk_prefix))) { > @@ -475,6 +493,14 @@ > return false; > } > } > + if (parse_ok == CONTROLLER_ATACAM) { > + if ((fdchan->camdev = ::cam_open_device(dev,O_RDWR)) == NULL) { > + perror("cam_open_device"); > + free(fdchan); > + errno = ENOENT; > + return false; > + } > + } > > if (parse_ok == CONTROLLER_3WARE_678K_CHAR) { > char buf[512]; > @@ -562,7 +588,10 @@ > if (fdchan->atacommand) > failed=::close(fdchan->atacommand); > #endif > - > + > + if (fdchan->camdev != NULL) > + cam_close_device(fdchan->camdev); > + > // if close succeeded, then remove from device list > // Eduard, should we also remove it from list if close() fails? I'm > // not sure. Here I only remove it from list if close() worked. > @@ -590,6 +619,10 @@ > > protected: > virtual int ata_command_interface(smart_command_set command, int select, char > * data); > + > + #ifdef IOCATAREQUEST > + virtual int do_cmd(struct freebsd_dev_channel* con, struct ata_ioc_request* > request); > + #endif > }; > > freebsd_ata_device::freebsd_ata_device(smart_interface * intf, const char * > dev_name, const char * req_type) > @@ -598,6 +631,72 @@ > { > } > > +int freebsd_ata_device::do_cmd(struct freebsd_dev_channel* con, struct > ata_ioc_request* request) > +{ > + return ioctl(con->device, IOCATAREQUEST, request); > +} > + > +#if __FreeBSD_version > 800100 > +class freebsd_atacam_device : public freebsd_ata_device > +{ > +public: > + freebsd_atacam_device(smart_interface * intf, const char * dev_name, const > char * req_type) > + : smart_device(intf, dev_name, "atacam", req_type), freebsd_ata_device(intf, > dev_name, req_type) > + {} > + > +protected: > + virtual int do_cmd(struct freebsd_dev_channel* con, struct ata_ioc_request* > request); > +}; > + > +int freebsd_atacam_device::do_cmd(struct freebsd_dev_channel* con, struct > ata_ioc_request* request) > +{ > + union ccb ccb; > + int camflags; > + > + memset(&ccb, 0, sizeof(ccb)); > + > + if (request->count == 0) > + camflags = CAM_DIR_NONE; > + else if (request->flags == ATA_CMD_READ) > + camflags = CAM_DIR_IN; > + else > + camflags = CAM_DIR_OUT; > + > + cam_fill_ataio(&ccb.ataio, > + 0, > + NULL, > + camflags, > + MSG_SIMPLE_Q_TAG, > + (u_int8_t*)request->data, > + request->count, > + request->timeout); > + > + // ata_28bit_cmd > + ccb.ataio.cmd.flags = 0; > + ccb.ataio.cmd.command = request->u.ata.command; > + ccb.ataio.cmd.features = request->u.ata.feature; > + ccb.ataio.cmd.lba_low = request->u.ata.lba; > + ccb.ataio.cmd.lba_mid = request->u.ata.lba >> 8; > + ccb.ataio.cmd.lba_high = request->u.ata.lba >> 16; > + ccb.ataio.cmd.device = 0x40 | ((request->u.ata.lba >> 24) & 0x0f); > + ccb.ataio.cmd.sector_count = request->u.ata.count; > + > + ccb.ccb_h.flags |= CAM_DEV_QFRZDIS; > + > + if (cam_send_ccb(con->camdev, &ccb) < 0) { > + err(1, "cam_send_ccb"); > + return -1; > + } > + > + if ((ccb.ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) > + return 0; > + > + cam_error_print(con->camdev, &ccb, CAM_ESF_ALL, CAM_EPF_ALL, stderr); > + return -1; > +} > + > +#endif > + > int freebsd_ata_device::ata_command_interface(smart_command_set command, int > select, char * data) > { > int fd=get_fd(); > @@ -741,8 +840,8 @@ > unsigned const char failed_lo=0xf4, failed_hi=0x2c; > unsigned char low,high; > > - #ifdef IOCATAREQUEST > - if ((retval=ioctl(con->device, IOCATAREQUEST, &request)) || request.error) > + #ifdef IOCATAREQUEST > + if ((retval=do_cmd(con, &request)) || request.error) > #else > if ((retval=ioctl(con->atacommand, IOCATA, &iocmd)) || request.error) > #endif > @@ -777,8 +876,8 @@ > return 0; > } > > - #ifdef IOCATAREQUEST > - if ((retval=ioctl(con->device, IOCATAREQUEST, &request)) || request.error) > + #ifdef IOCATAREQUEST > + if ((retval=do_cmd(con, &request)) || request.error) > #else > if ((retval=ioctl(con->atacommand, IOCATA, &iocmd)) || request.error) > #endif > @@ -1571,6 +1670,8 @@ > protected: > virtual ata_device * get_ata_device(const char * name, const char * type); > > + virtual ata_device * get_atacam_device(const char * name, const char * type); > + > virtual scsi_device * get_scsi_device(const char * name, const char * type); > > virtual smart_device * autodetect_smart_device(const char * name); > @@ -1604,6 +1705,11 @@ > return new freebsd_ata_device(this, name, type); > } > > +ata_device * freebsd_smart_interface::get_atacam_device(const char * name, > const char * type) > +{ > + return new freebsd_atacam_device(this, name, type); > +} > + > scsi_device * freebsd_smart_interface::get_scsi_device(const char * name, const > char * type) > { > return new freebsd_scsi_device(this, name, type); > @@ -2162,6 +2268,8 @@ > switch (guess) { > case CONTROLLER_ATA : > return new freebsd_ata_device(this, name, ""); > + case CONTROLLER_ATACAM : > + return new freebsd_atacam_device(this, name, ""); > case CONTROLLER_SCSI: > // Try to detect possible USB->(S)ATA bridge > if (get_usb_id(name, vendor_id, product_id, version)) { > Index: os_freebsd.h > =================================================================== > --- os_freebsd.h (revision 2902) > +++ os_freebsd.h (working copy) > @@ -92,6 +92,7 @@ > #endif > char* devname; // the SCSI device name > int unitnum; // the SCSI unit number > + struct cam_device *camdev; > }; > > #define FREEBSD_MAXDEV 64 > > |
From: Andriy G. <av...@ic...> - 2009-09-07 17:03:53
|
on 07/09/2009 19:52 Alex Samorukov said the following: > Andriy Gapon wrote: >> Latest FreeBSD has an option of using ahci(4) driver which provides >> access to >> SATA disks via CAM. In that case the disk devices are named ada*, they >> do not >> support ATA ioctls and ATA commands should be sent via cam(3). >> >> Here is the first take at implementing support for ada devices. >> Auto-discovery of ada devices is not implemented yet. It should be >> fairly easy >> to do using camcontrol.c code as an example. >> Code in freebsd_atacam_device::do_cmd should really use more of atacam >> helper >> functions. >> >> I am not sure if my approach of making freebsd_atacam_device a >> subclass of >> freebsd_ata_device is such a good idea, but I took it for a fast >> re-use of >> ata_command_interface method. >> >> > Hello Andriy, > > Thank you for your patches. What do you think - may be its better to > handle such devices (initially) as scsi, since we already having code > for cam_open, cam_close, etc? Alex, it's a hard question, because ada devices are a sort of cross-over between SCSI and ATA - they use the same transport for command delivery as SCSI (that is, CAM) and the use the same ATA commands. So this new class needs I/O code from existing SCSI class and command building code from ATA class. > Auto discovery already implemented for cam devices, so it should not be > a problem to extend existing code to support ada* devices. Yes. Basically the only difference is PROTO_ATA vs PROTO_SCSI. > If you want me to commit this patches after review - please, open the > ticket and attach patch to it. Well, I am not sure if this code is sufficiently ready yet. -- Andriy Gapon |
From: Alex S. <ml...@os...> - 2009-09-07 17:33:25
|
Andriy Gapon wrote: >> >> Thank you for your patches. What do you think - may be its better to >> handle such devices (initially) as scsi, since we already having code >> for cam_open, cam_close, etc? >> > > Alex, > > it's a hard question, because ada devices are a sort of cross-over between SCSI > and ATA - they use the same transport for command delivery as SCSI (that is, CAM) > and the use the same ATA commands. So this new class needs I/O code from existing > SCSI class and command building code from ATA class. > Yes, I see. Possibly we need to make some basic functions for cam and call them from scsi and atacam classes? This will allow us to avoid unneeded code duplication. Also do you know if such devices are creating pass* device nodes? And is it possible to handle them with smartmontools? > Well, I am not sure if this code is sufficiently ready yet. > It is not yet (at least "-d atapicam" should be added, manual updated, etc.), but IMHO it is easer to work inside ticket interface then to post sources in the maillist. |
From: Andriy G. <av...@ic...> - 2009-09-07 17:47:35
|
on 07/09/2009 20:33 Alex Samorukov said the following: > Yes, I see. Possibly we need to make some basic functions for cam and > call them from scsi and atacam classes? This will allow us to avoid > unneeded code duplication. Yes, good idea. BTW, I see that right now SCSI class opens and closes a device for every command. See cam_open_spec_device in do_normal_scsi_cmnd_io. This is OK, but not optimal. > Also do you know if such devices are creating > pass* device nodes? And is it possible to handle them with smartmontools? Yes, they do create pass* devices, cam(3) works though those [*]. And, yes, it's possible to use them in smartmontools, this is what the patch does :-) Or maybe I didn't get what you were actually asking. [*] cam(3), pass(4), xpt(4) P.S. pass(4) has some words that over-signify SCSI devices: "The pass driver attaches to every SCSI device...". Actually pass driver attaches to every CAM-managed device. -- Andriy Gapon |
From: Alexander M. <mav@FreeBSD.org> - 2009-09-07 18:51:29
|
Alex Samorukov wrote: > Yes, I see. Possibly we need to make some basic functions for cam and > call them from scsi and atacam classes? This will allow us to avoid > unneeded code duplication. Also do you know if such devices are creating > pass* device nodes? And is it possible to handle them with smartmontools? Yes. These devices are very alike to SCSI disks and also have pass* devices. The only difference is using other type of CCB ccb_ataio for wrapping ATA command instead of ccb_scsiio for SCSI. For examples you may look on updated camcontrol tool sources: http://www.freebsd.org/cgi/cvsweb.cgi/src/sbin/camcontrol/camcontrol.c , look for cam_fill_ataio(). -- Alexander Motin |
From: Dan L. <da...@ob...> - 2009-09-07 17:57:17
|
Andriy Gapon napsal/wrote, On 09/07/09 19:03: > on 07/09/2009 19:52 Alex Samorukov said the following: >>> I am not sure if my approach of making freebsd_atacam_device a >>> subclass of >>> freebsd_ata_device is such a good idea, but I took it for a fast >>> re-use of >>> ata_command_interface method. >> Thank you for your patches. What do you think - may be its better to >> handle such devices (initially) as scsi, since we already having code >> for cam_open, cam_close, etc? > > Alex, > > it's a hard question, because ada devices are a sort of cross-over between SCSI > and ATA - they use the same transport for command delivery as SCSI (that is, CAM) > and the use the same ATA commands. So this new class needs I/O code from existing > SCSI class and command building code from ATA class. In the fact, the ADA is not the only device that use ATA-syntax command, but non-ATA transport layer. The SAT is other device that send's SAT-encapsulated ATA commands using SCSI ioctls, areca controllers (i'm ready to add support for such controller, but I'm waiting until code rewritten to new style) send's ARECA-encapsulated then SAT-encapsulated ATA commands using SCSI ioctl. 3ware sends ATA commands with proprietary encapsulation using ATA ioctl ... I'm sure we don't want to have same code repeated in different device-driver classes (SAT encapsulation in SAT and ARECA, SCSI ioctls handling in many classes, compiling of ATA command structures in many classes). Alex anounced he is working on rewrite of os_freebsd.cpp from "old style" to "new style". IMHO, we should wait (you with ada and me with areca) until major part of rewriting will be done. Then we should write the support for new device with new interface. Dan |
From: Andriy G. <av...@ic...> - 2009-09-07 17:59:39
|
on 07/09/2009 20:56 Dan Lukes said the following: > IMHO, we should wait (you with ada and me with areca) until major part > of rewriting will be done. Then we should write the support for new > device with new interface. I think it's already in new style with classes and all, no? I am talking of svn trunk version. -- Andriy Gapon |
From: Dan L. <da...@ob...> - 2009-09-07 18:06:19
|
Andriy Gapon napsal/wrote, On 09/07/09 19:59: > on 07/09/2009 20:56 Dan Lukes said the following: >> IMHO, we should wait (you with ada and me with areca) until major part >> of rewriting will be done. Then we should write the support for new >> device with new interface. > > I think it's already in new style with classes and all, no? > I am talking of svn trunk version. So I. The code has "new style" interface presented to the calling code. But internally is still "old". See several comments containing string "old function". Dan |
From: Andriy G. <av...@ic...> - 2009-09-07 18:10:58
|
on 07/09/2009 21:06 Dan Lukes said the following: > Andriy Gapon napsal/wrote, On 09/07/09 19:59: >> on 07/09/2009 20:56 Dan Lukes said the following: >>> IMHO, we should wait (you with ada and me with areca) until major part >>> of rewriting will be done. Then we should write the support for new >>> device with new interface. >> >> I think it's already in new style with classes and all, no? >> I am talking of svn trunk version. > > So I. > > The code has "new style" interface presented to the calling code. But > internally is still "old". See several comments containing string "old > function". Got you now. Thank you for making me informed! :) -- Andriy Gapon |
From: Alex S. <ml...@os...> - 2009-09-09 07:28:31
|
Dan Lukes wrote: > Andriy Gapon napsal/wrote, On 09/07/09 19:59: > >> on 07/09/2009 20:56 Dan Lukes said the following: >> >>> IMHO, we should wait (you with ada and me with areca) until major part >>> of rewriting will be done. Then we should write the support for new >>> device with new interface. >>> >> I think it's already in new style with classes and all, no? >> I am talking of svn trunk version. > > > The code has "new style" interface presented to the calling code. But > internally is still "old". See several comments containing string "old > function". > > To be more correct - it is "in the middle". What I want to rewrite is detection part (too many functions for the same things, "goto", complicated logic, etc.). But I`m working on this on my free time which is limited now, so I would like to recommend to open TRAC tickets for the new hardware. This will allow me, from one side, keep in mind that there are such devices (and possibly - integrate them), and also it will allow to work on them before everything done. |
From: Dan L. <da...@ob...> - 2009-09-09 07:53:57
|
Alex Samorukov napsal/wrote, On 09/09/09 09:28: >> The code has "new style" interface presented to the calling code. But >> internally is still "old". See several comments containing string "old >> function". > To be more correct - it is "in the middle". What I want to rewrite is > detection part (too many functions for the same things, "goto", > complicated logic, etc.). But I`m working on this on my free time which > is limited now, so I would like to recommend to open TRAC tickets for > the new hardware. This will allow me, from one side, keep in mind that > there are such devices (and possibly - integrate them), and also it will > allow to work on them before everything done. No problem. I can commit the support for the new controller by myself, in my own free time. But I don't want to write them just now, for the current "old style coding". It's sounds to me that not the best time to add code that must be rewriten in short horizont. We are not pushing you to the limits. I will wait patiently until your's work will be done. Dan |