Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 25 Mar 2012 22:35:33 -0700
From:      Cy Schubert <Cy.Schubert@komquats.com>
To:        Marius Strobl <marius@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r233274 - in head/sys/dev/ata: . chipsets
Message-ID:  <201203260535.q2Q5ZXE7043378@slippy.cwsent.com>
In-Reply-To: Your message of "Wed, 21 Mar 2012 08:57:15 -0000." <201203210857.q2L8vFLB062984@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
In message <201203210857.q2L8vFLB062984@svn.freebsd.org>, Marius Strobl 
writes:
> Author: marius
> Date: Wed Mar 21 08:57:15 2012
> New Revision: 233274
> URL: http://svn.freebsd.org/changeset/base/233274
> 
> Log:
>   Remove remnants of ATA_LOCKING uses in the ATA_CAM case and wrap it
>   along with functions, SYSCTLs and tunables that are not used with
>   ATA_CAM in #ifndef ATA_CAM, similar to the existing #ifdef'ed ATA_CAM
>   code for the other way around. This makes it easier to understand
>   which parts of ata(4) actually are used in the new world order and
>   to later on remove the !ATA_CAM bits. It also makes it obvious that
>   there is something fishy with the C-bus front-end as well as in the
>   ATP850 support, as these used ATA_LOCKING which is defunct in the
>   ATA_CAM case. When fixing the former, ATA_LOCKING probably needs to
>   be brought back in some form or other.
>   
>   Reviewed by:	mav
>   MFC after:	1 week
> 
> Modified:
>   head/sys/dev/ata/ata-all.c
>   head/sys/dev/ata/ata-cbus.c
>   head/sys/dev/ata/ata-pci.c
>   head/sys/dev/ata/ata-pci.h
>   head/sys/dev/ata/ata-queue.c
>   head/sys/dev/ata/chipsets/ata-acard.c
> 
[... diff removed for brevity ...]

Hi,

This commit broke kernels with device atapicam specified:

# ATA and ATAPI devices
device          atapicam        # emulate ATAPI devices as SCSI ditto via 
CAM
                                        # needs CAM to be present (scbus & 
pass)

Here are two examples when device atapicam is specified in the kernel 
config:

cd /usr/obj/dsk03/src/svn-current/sys/BREAK; MAKEOBJDIRPREFIX=/usr/obj  
MACHINE_ARCH=i386  MACHINE=i386  CPUTYPE= GROFF_BIN_PATH=/usr/obj/dsk03/src/
svn-current/tmp/legacy/usr/bin  GROFF_FONT_PATH=/usr/obj/dsk03/src/svn-curre
nt/tmp/legacy/usr/share/groff_font  GROFF_TMAC_PATH=/usr/obj/dsk03/src/svn-c
urrent/tmp/legacy/usr/share/tmac  _SHLIBDIRPREFIX=/usr/obj/dsk03/src/svn-cur
rent/tmp  VERSION="FreeBSD 10.0-CURRENT i386 1000009"  INSTALL="sh 
/dsk03/src/svn-current/tools/install.sh"  PATH=/usr/obj/dsk03/src/svn-curren
t/tmp/legacy/usr/sbin:/usr/obj/dsk03/src/svn-current/tmp/legacy/usr/bin:/usr
/obj/dsk03/src/svn-current/tmp/legacy/usr/games:/usr/obj/dsk03/src/svn-curre
nt/tmp/usr/sbin:/usr/obj/dsk03/src/svn-current/tmp/usr/bin:/usr/obj/dsk03/sr
c/svn-current/tmp/usr/games:/sbin:/bin:/usr/sbin:/usr/bin make 
KERNEL=kernel all -DNO_MODULES_OBJ
linking kernel.debug
atapi-cam.o: In function `atapi_action':
/dsk03/src/svn-current/sys/dev/ata/atapi-cam.c:436: undefined reference to 
`ata_controlcmd'
/dsk03/src/svn-current/sys/dev/ata/atapi-cam.c:651: undefined reference to 
`ata_queue_request'
*** [kernel.debug] Error code 1

Stop in /dsk02/obj/dsk03/src/svn-current/sys/BREAK.
*** [buildkernel] Error code 1

Stop in /dsk03/src/svn-current.
*** [buildkernel] Error code 1

Stop in /dsk03/src/svn-current.
bob# 

	... and later ...

cc -c -O -pipe  -std=c99 -g -Wall -Wredundant-decls -Wnested-externs 
-Wstrict-prototypes  -Wmissing-prototypes -Wpointer-arith -Winline 
-Wcast-qual  -Wundef -Wno-pointer-sign -fformat-extensions  
-Wmissing-include-dirs -fdiagnostics-show-option   -nostdinc  -I. 
-I/dsk03/src/svn-current/sys -I/dsk03/src/svn-current/sys/contrib/altq 
-D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h -fno-common 
-finline-limit=8000 --param inline-unit-growth=100 --param 
large-function-growth=1000  -mno-align-long-strings 
-mpreferred-stack-boundary=2 -mno-mmx -mno-sse -msoft-float -ffreestanding 
-fstack-protector -Werror  vers.c
linking kernel.debug
ata-queue.o: In function `ata_controlcmd':
/dsk03/src/svn-current/sys/dev/ata/ata-queue.c:154: undefined reference to 
`ata_queue_request'
ata-queue.o: In function `ata_atapicmd':
/dsk03/src/svn-current/sys/dev/ata/ata-queue.c:177: undefined reference to 
`ata_queue_request'
atapi-cam.o: In function `atapi_action':
/dsk03/src/svn-current/sys/dev/ata/atapi-cam.c:651: undefined reference to 
`ata_queue_request'
*** [kernel.debug] Error code 1

Stop in /dsk02/obj/dsk03/src/svn-current/sys/BREAK.
*** [buildkernel] Error code 1

Stop in /dsk03/src/svn-current.
*** [buildkernel] Error code 1

Stop in /dsk03/src/svn-current.
bob# 

There are many more like this.

And, the patch to fix the issue:

Index: ata-queue.c
===================================================================
--- ata-queue.c	(revision 233460)
+++ ata-queue.c	(working copy)
@@ -43,14 +43,11 @@
 #include <dev/ata/ata-all.h>
 #include <ata_if.h>
 
-#ifndef ATA_CAM
 /* prototypes */
 static void ata_completed(void *, int);
 static void ata_sort_queue(struct ata_channel *ch, struct ata_request 
*request);
 static const char *ata_skey2str(u_int8_t);
-#endif
 
-#ifndef ATA_CAM
 void
 ata_queue_request(struct ata_request *request)
 {
@@ -126,9 +123,7 @@
 	sema_destroy(&request->done);
     }
 }
-#endif
 
-#ifndef ATA_CAM
 int
 ata_controlcmd(device_t dev, u_int8_t command, u_int16_t feature,
 	       u_int64_t lba, u_int16_t count)
@@ -158,9 +153,7 @@
     }
     return error;
 }
-#endif
 
-#ifndef ATA_CAM
 int
 ata_atapicmd(device_t dev, u_int8_t *ccb, caddr_t data,
 	     int count, int flags, int timeout)
@@ -183,9 +176,7 @@
     }
     return error;
 }
-#endif
 
-#ifndef ATA_CAM
 void
 ata_start(device_t dev)
 {
@@ -244,9 +235,7 @@
 	}
     }
 }
-#endif
 
-#ifndef ATA_CAM
 void
 ata_finish(struct ata_request *request)
 {
@@ -274,9 +263,7 @@
 	}
     }
 }
-#endif
 
-#ifndef ATA_CAM
 static void
 ata_completed(void *context, int dummy)
 {
@@ -508,7 +495,6 @@
     if (ch)
 	ata_start(ch->dev);
 }
-#endif
 
 void
 ata_timeout(struct ata_request *request)
@@ -605,7 +591,6 @@
 }
 #endif
 
-#ifndef ATA_CAM
 static u_int64_t
 ata_get_lba(struct ata_request *request)
 {
@@ -627,9 +612,7 @@
     else
 	return request->u.ata.lba;
 }
-#endif
 
-#ifndef ATA_CAM
 static void
 ata_sort_queue(struct ata_channel *ch, struct ata_request *request)
 {
@@ -682,7 +665,6 @@
 	ch->freezepoint = request;
     TAILQ_INSERT_AFTER(&ch->ata_queue, this, request, chain);
 }
-#endif
 
 const char *
 ata_cmd2str(struct ata_request *request)
@@ -798,7 +780,6 @@
     return buffer;
 }
 
-#ifndef ATA_CAM
 static const char *
 ata_skey2str(u_int8_t skey)
 {
@@ -822,4 +803,3 @@
     default: return("UNKNOWN");
     }
 }
-#endif


-- 
Cheers,
Cy Schubert <Cy.Schubert@komquats.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  http://www.FreeBSD.org

	The need of the many outweighs the greed of the few.






Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203260535.q2Q5ZXE7043378>