From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751352AbdBNXnM (ORCPT ); Tue, 14 Feb 2017 18:43:12 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:49174 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750816AbdBNXnK (ORCPT ); Tue, 14 Feb 2017 18:43:10 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org ED30E60ACA Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=cov@codeaurora.org Subject: Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit To: Timur Tabi , Jonathan Corbet , Marc Zyngier , Catalin Marinas , Will Deacon , linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Mark Rutland , linux-kernel@vger.kernel.org, shankerd@codeaurora.org, Greg Kroah-Hartman , Jiri Slaby , Russell King , linux-serial@vger.kernel.org References: <20170208005736.18724-1-cov@codeaurora.org> <6afb0a98-0973-33ec-0b76-fc6378e64309@codeaurora.org> <5e18324f-d09d-d652-df23-d4a3f8f053fd@codeaurora.org> Cc: Mark Langsdorf , Mark Salter , Jon Masters , Neil Leeder From: Christopher Covington Message-ID: <5b771f8f-5a39-4c57-34a1-8e3f6d729a11@codeaurora.org> Date: Tue, 14 Feb 2017 18:43:06 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2 MIME-Version: 1.0 In-Reply-To: <5e18324f-d09d-d652-df23-d4a3f8f053fd@codeaurora.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/08/2017 06:04 PM, Timur Tabi wrote: > On 02/08/2017 04:22 PM, Christopher Covington wrote: >>>> >> - while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy) >>>> >> + while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr) >>>> >> + & uap->vendor->fr_busy) >>> > >>> > I really think the XOR logic needs to be documented wherever it's >>> > used. It's just too confusing. >> References such as the following for basic bit operations like >> setting, clearing, and toggling are easy enough to come by. >> >> http://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit-in-c-c > > I know what XOR does. I was thinking a more high-level comment would > be appropriate. With E44, we want to ignore the BUSY bit and use TXFE > instead. So was hoping to a comment that says that: > > /* Normally, we poll until BUSY=0, but E44 says we should poll until > TXFE=1 instead. So with E44, we set fr_busy to TXFE, but we have to > invert it. */ Oh, yeah, that'd be good. I'll include better comments in v2. Cheers, Cov -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. From mboxrd@z Thu Jan 1 00:00:00 1970 From: cov@codeaurora.org (Christopher Covington) Date: Tue, 14 Feb 2017 18:43:06 -0500 Subject: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit In-Reply-To: <5e18324f-d09d-d652-df23-d4a3f8f053fd@codeaurora.org> References: <20170208005736.18724-1-cov@codeaurora.org> <6afb0a98-0973-33ec-0b76-fc6378e64309@codeaurora.org> <5e18324f-d09d-d652-df23-d4a3f8f053fd@codeaurora.org> Message-ID: <5b771f8f-5a39-4c57-34a1-8e3f6d729a11@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 02/08/2017 06:04 PM, Timur Tabi wrote: > On 02/08/2017 04:22 PM, Christopher Covington wrote: >>>> >> - while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy) >>>> >> + while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr) >>>> >> + & uap->vendor->fr_busy) >>> > >>> > I really think the XOR logic needs to be documented wherever it's >>> > used. It's just too confusing. >> References such as the following for basic bit operations like >> setting, clearing, and toggling are easy enough to come by. >> >> http://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit-in-c-c > > I know what XOR does. I was thinking a more high-level comment would > be appropriate. With E44, we want to ignore the BUSY bit and use TXFE > instead. So was hoping to a comment that says that: > > /* Normally, we poll until BUSY=0, but E44 says we should poll until > TXFE=1 instead. So with E44, we set fr_busy to TXFE, but we have to > invert it. */ Oh, yeah, that'd be good. I'll include better comments in v2. Cheers, Cov -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.