From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753122AbXCaOWN (ORCPT ); Sat, 31 Mar 2007 10:22:13 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753118AbXCaOWN (ORCPT ); Sat, 31 Mar 2007 10:22:13 -0400 Received: from hancock.steeleye.com ([71.30.118.248]:38413 "EHLO hancock.sc.steeleye.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753109AbXCaOWM (ORCPT ); Sat, 31 Mar 2007 10:22:12 -0400 Subject: Re: [PATCH 1/4] [SCSI]stex: fix id mapping issue From: James Bottomley To: Ed Lin Cc: linux-scsi , linux-kernel , jeff , promise_linux In-Reply-To: References: Content-Type: text/plain Date: Sat, 31 Mar 2007 09:22:08 -0500 Message-Id: <1175350928.3760.55.camel@mulgrave.il.steeleye.com> Mime-Version: 1.0 X-Mailer: Evolution 2.8.3 (2.8.3-1.fc6) Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2007-03-30 at 15:21 -0700, Ed Lin wrote: > The internal id/lun mapping of st_vsc and st_vsc1 controllers is different > from st_shasta. The original driver code can only map first 16 'entities' > for st_vsc and st_vsc1 while there are actually 128 available. > > Also the ST_MAX_LUN_PER_TARGET should be 8, although this can do > no harm because inquiries beyond boundary are discarded by firmware. > > The correct internal mapping should be: > id:0~15, lun:0~7 (st_shasta) > id:0, lun:0~127 (st_yosemite) > id:0~127, lun:0 (st_vsc and st_vsc1) > To scsi mid layer they are all channel:0~7, id:0~15, lun:0, with a maximun > 'entity' number of 128. The RAID console only interfaces to scsi mid layer > and is always mapped at channel:0, id:16, lun:0. I'm with Christoph here ... if we're going to break the backwards compatibility of the mappings (which your code does) then we could just dump channel and use the SCSI id and lun directly. Understanding this code is predicated on this quirky definition in stex_queuecommand: id = cmd->device->id; lun = cmd->device->channel; /* firmware lun issue work around */ ^^^^^^^ > @ -645,12 +645,16 @@ stex_queuecommand(struct scsi_cmnd *cmd, > > req = stex_alloc_req(hba); > > - if (hba->cardtype == st_yosemite) { > - req->lun = lun * (ST_MAX_TARGET_NUM - 1) + id; This looks to be correct, it goes up id 0 to ST_MAX_TARGET_NUM -1 then takes the next channel. > - req->target = 0; > - } else { > + if (hba->cardtype == st_shasta) { > req->lun = lun; > req->target = id; > + } else if (hba->cardtype == st_yosemite){ > + req->lun = id * ST_MAX_LUN_PER_TARGET + lun; > + req->target = 0; > + } else { > + /* st_vsc and st_vsc1 */ > + req->lun = 0; > + req->target = id * ST_MAX_LUN_PER_TARGET + lun; These both look to be wrong. You're taking the channel as the lowest common denominator, so your first target is on channel 1 id 0, your next on channel 2, id 0 and so on. That's really going to mess with the ordering (which will be user visible) is that really what you want? James