From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Subject: Re: [PATCH 06/12] sata_sil: convert to new reset mechanism Date: Sun, 29 Jan 2006 08:17:04 +0900 Message-ID: <43DBFB70.4050002@gmail.com> References: <1138089922913-git-send-email-htejun@gmail.com> <43DBB34B.1050302@pobox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from xproxy.gmail.com ([66.249.82.206]:15119 "EHLO xproxy.gmail.com") by vger.kernel.org with ESMTP id S1750778AbWA1XRJ (ORCPT ); Sat, 28 Jan 2006 18:17:09 -0500 Received: by xproxy.gmail.com with SMTP id s14so539828wxc for ; Sat, 28 Jan 2006 15:17:08 -0800 (PST) In-Reply-To: <43DBB34B.1050302@pobox.com> Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Jeff Garzik Cc: linux-ide@vger.kernel.org, albertcc@tw.ibm.com Jeff Garzik wrote: > Tejun Heo wrote: > >> Convert sata_sil to use new reset mechanism. sata_sil is fairly >> generic and can directly use std routine. >> >> Signed-off-by: Tejun Heo > > > Major NAK. > > The placement of every hardware register read/write has been very > carefully laid out, and your reset hardware re-orders a lot of that. > Code re-ordering is fine. Changing the order of I/O operations is not. > With this patch, sata_sil fails to wake up and initialize the SATA phy! > > Proper code transformation is art. You're doing really well, but this > was a bit of a hiccup. You should have presented a series of patches > which changed over to your new ata_drive_probe_reset() stuff, while -not > changing the hardware operations at all-. Like a mathematical solution, > your code is then provably correct. If there is a regression, then you > know it is a driver bug, and not a problem with hardware acting weird. > > If the driver can access the SATA PHY registers, the very first thing it > should usually do is the code in sata_std_hardreset()... everything > except the 0x301 value write. Or in other words, follow the code in > __sata_phy_reset() PRECISELY (presuming that ATA_FLAG_SATA_RESET is not > set). > > sata_sil hardware initialization order and behavior should not change at > all [though certainly you will change it sometime in the future]. Okay, I see. I'll rewrite those parts such that it acts in the same way as the original reset. > So... I will drop all other patches from you, let you resync with the > latest stuff in libata-dev.git#upstream (NOTE: renamed from > upstream-2.6.17 to upstream), and wait for your next barrage of patches ;-) It's lunar new year here and I am away from office until 1st Feb, so it will take me some time. Thanks a lot for all your help and guidance. It's been a great year. Happy (lunar) new year, guys. :-) -- tejun