Hi Bernd, Response inline Thanks Sathya -----Original Message----- From: Bernd Schubert [mailto:bs@xxxxxxxxx] Sent: Thursday, October 30, 2008 9:48 PM To: Prakash, Sathya Cc: Linux SCSI Mailing List; Moore, Eric; James Bottomley; DL-MPT Fusion Linux Subject: Re: [ PATCH 1/4 ] mpt fusion SoftReset handler Hello Sathya, thanks for review! See below for my comments. On Thursday 30 October 2008 16:37:33 Prakash, Sathya wrote: > Bernd, > My comments are inlined with /*SP. Except those the patch looks good > Thanks Sathya > + for (cb_idx = MPT_MAX_PROTOCOL_DRIVERS-1; cb_idx; cb_idx--) { > + if (MptResetHandlers[cb_idx]) > + mpt_signal_reset(cb_idx, ioc, MPT_IOC_SETUP_RESET); > + } > + > > /*SP: Move the following PRE_RESET signal after issuing the MUR. This > will cause the firmware to fault sometime */ > > + for (cb_idx = MPT_MAX_PROTOCOL_DRIVERS-1; cb_idx; cb_idx--) { > + if (MptResetHandlers[cb_idx]) > + mpt_signal_reset(cb_idx, ioc, MPT_IOC_PRE_RESET); > + } > + > + /* Disable reply interrupts (also blocks FreeQ) */ > + CHIPREG_WRITE32(&ioc->chip->IntMask, 0xFFFFFFFF); > + ioc->active = 0; > + time_count = jiffies; > + rc = SendIocReset(ioc, MPI_FUNCTION_IOC_MESSAGE_UNIT_RESET, > sleepFlag); + if (rc != 0) > + goto out; You mean mpt_signal_reset(cb_idx, ioc, MPT_IOC_PRE_RESET); here? But at this point we already did a reset, so MPT_IOC_PRE_RESET seems a bit odd. But if you are sure... (I really wish I had the MPT docs). [...] /* Yes Here. There name PRE_RESET is confusing, if you see the action inside PRE_RESET in mptscsih.c it clears the pending requests issued to firmware. Before sending the MUR to FW. There is possibility that when firmware before processing the MUR tries to find the request to do DMA and couldn't find that because the driver cleared it already in PRE_RESET and hence resulted in fault. > > @@ -680,6 +680,7 @@ static int mptctl_do_reset(unsigned long > dctlprintk(iocp, printk(MYIOC_s_DEBUG_FMT "mptctl_do_reset > called.\n", iocp->name)); > > + /* FIXME: Can we call mptSoftHardResetHandler() here? */ > /*SP No It should be retained as a last option to clear firmware > faults using utilities */ if (mpt_HardResetHandler(iocp, CAN_SLEEP) != > 0) { Ok, I will remove this hunk then. Cheers, Bernd -- Bernd Schubert Q-Leap Networks GmbH -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
