From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58210) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XoAhO-0006Ks-Ta for qemu-devel@nongnu.org; Tue, 11 Nov 2014 07:37:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XoAhH-0003AN-0O for qemu-devel@nongnu.org; Tue, 11 Nov 2014 07:37:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52043) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XoAhG-0003AJ-Oe for qemu-devel@nongnu.org; Tue, 11 Nov 2014 07:37:22 -0500 Message-ID: <546202FD.7060605@redhat.com> Date: Tue, 11 Nov 2014 13:37:17 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1415634775-4288-1-git-send-email-hare@suse.de> <546104B3.6020707@redhat.com> <54611BD9.6020105@suse.de> In-Reply-To: <54611BD9.6020105@suse.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv2] esp: Do not overwrite ESP_TCHI after reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hannes Reinecke Cc: qemu-devel@nongnu.org On 10/11/2014 21:11, Hannes Reinecke wrote: > On 11/10/2014 07:32 PM, Paolo Bonzini wrote: >> On 10/11/2014 16:52, Hannes Reinecke wrote: >>> After a reset ESP_TCHI should contain the unique ID >>> of the chip. This value will be overwritten with the >>> current tranfer count if the transfer count has >>> previously been set. >>> So we should always return the chip id if ESP_TCHI >>> has never been written to. >> >> What if ESP_TCHI was written 0? Why should it return the chip id? >> > It's a complex thing. The documentation says 'ESP_TCHI returns the chip > id until been written to'. > And ESP_TCHI is strictly speaking only valid if the 'Features enabled' > bit is set in ESP_CFG2. (Not that the driver checks this). > To handle it correctly we would need to add a flag whenever ESP_TCHI > is written to, but I thought it'd be slightly too much. I'm worried that the chip_id will reappear if you write a low (<65536) value to ESP_TCLOW/MED/HI. Adding the flag is just a couple lines more than this patch, let's do it right. Paolo