Date: Thu, 24 Aug 2000 19:33:02 -0700 From: Mike Smith <msmith@freebsd.org> To: Katsushi Kobayashi <ikob@koganei.wide.ad.jp> Cc: freebsd-hackers@freebsd.org Subject: Re: IEEE1394 driver system for -current Message-ID: <200008250233.TAA00857@mass.osd.bsdi.com> In-Reply-To: Your message of "Sat, 19 Aug 2000 19:00:26 %2B0900." <399E5A33.B552E780@koganei.wide.ad.jp>
next in thread | previous in thread | raw e-mail | index | archive | help
> I announced IEEE1394 device driver on FREENIX'99 (sorry not > on '00). I have caught up -current version at this time. > > The latest -current driver patch can be found at: > > ftp://ftp.uec.ac.jp/pub/firewire/beta/ > > I hope you success to make a kernel on your source tree. Thanks again for the reminder about this. I've taken some time to play a little with the code and some hardware, and I have some comments on the code and what needs to be changed before it would be suitable for committing. > The driver function is still limited and may include many > bugs since the driver has been used for specific purrposes, > e.g., the driver have not supported SCSI (CAM) storage on > IEEE1394 and not been complient to loadable kernel module. > However, I think it is better to merge -current on this time and > maintain on it compared with taking a effort in independent. > > Let me know what shall I do to merge my code. There are several things that need to be fixed. I'm not going to be nice about the code, because it needs some major work before it is ready, but I do think that it is worth using your code (and your skills) in some form. 1) The code is very messy. This makes it hard to read. It needs to be reformatted before committing to reduce whitespace/style changes afterwards. 2) There are a lot of "magic numbers" (numeric values for registers and so forth) rather than defined constants. This makes it hard to work out what parts of the code are doing. 3) The code lacks structure. There are some very clear divisions that should be made between the various components in the 1394 stack, and these are not being made. Fixing this will involve moving a lot of code around, and should be done before the code is committed. 1394 can be looked at as being quite similar to USB (the predominant interface model is OHCI, also used for USB). The obvious software components are: a) Host adapter driver (eg. TI Lynx, NEC, Adaptec, etc.) b) Generic 1394 layer c) Peripheral/protocol driver layer (eg. DV, CAM shim, etc) c') Layered peripheral/protocol driver layer The interface between layers a) and b), and between layers b) and c) and between layers c) and c') should all be clearly defined. Using newbus and defining these interactions in terms of parent-child relationships and bus methods will make this very easy. Having said so many cruel things about your code, it's clear that you've spent a lot of time making your stack work, and during that time you must have accumulated a lot of experience with these peripherals. I understand that Warner has proposed you be granted CVS commit access in order to work on your code in the FreeBSD CVS repository, and I'm in favour of this. I would, however, like to see the above issues addressed before the initial import. One option that you might want to consider is working with Bill Paul on the cleanup and restructuring process. Bill has a lot of experience with network-like peripherals, and with the way in which we like things to be done. In combination with your experience with 1394 in general, I think this would result in some very good and useful code. I've already spoken to Bill about this, and he seemed interested. (Apologies if I'm dumping you in it here, pal. 8) In summary, then, I would encourage you to consider the points above, and engage in some discussion (either on the -arch list or on a new freebsd-1394 list) about how to go about reorganising and cleaning up your code ready for committing as soon as practical. Regards, -- ... every activity meets with opposition, everyone who acts has his rivals and unfortunately opponents also. But not because people want to be opponents, rather because the tasks and relationships force people to take different points of view. [Dr. Fritz Todt] To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200008250233.TAA00857>