Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S942317AbcJSO0Y (ORCPT ); Wed, 19 Oct 2016 10:26:24 -0400 Received: from vader.hardeman.nu ([95.142.160.32]:52886 "EHLO hardeman.nu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S941831AbcJSO0U (ORCPT ); Wed, 19 Oct 2016 10:26:20 -0400 Mime-Version: 1.0 Date: Wed, 19 Oct 2016 13:10:31 +0000 Content-Type: text/plain; charset="utf-8" Message-ID: <403be317930e0915cbe98c15cd6adf66@hardeman.nu> X-Mailer: RainLoop/1.10.3.151 From: "=?utf-8?B?RGF2aWQgSMOkcmRlbWFu?=" Subject: Re: [PATCH 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection To: "SF Markus Elfring" , "Sean Young" Cc: linux-media@vger.kernel.org, "Mauro Carvalho Chehab" , "LKML" , kernel-janitors@vger.kernel.org, "Julia Lawall" In-Reply-To: <7fe65702-ac76-39f2-edea-eba007a3ee96@users.sourceforge.net> References: <7fe65702-ac76-39f2-edea-eba007a3ee96@users.sourceforge.net> <566ABCD9.1060404@users.sourceforge.net> <1d7d6a2c-0f1e-3434-9023-9eab25bb913f@users.sourceforge.net> <84757ae3-24d2-cf9b-2217-fd9793b86078@users.sourceforge.net> <20161015132956.GA3393@gofer.mess.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u9JET1f2016784 Content-Length: 1215 Lines: 31 October 15, 2016 6:42 PM, "SF Markus Elfring" wrote: >>> + /* Set CEIR_EN */ >>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01); >>> +set_irqmask: >>> /* >>> * ACPI will set the HW disable bit for SP3 which means that the >>> * output signals are left in an undefined state which may cause >>> @@ -876,6 +858,14 @@ wbcir_shutdown(struct pnp_dev *device) >>> */ >>> wbcir_set_irqmask(data, WBCIR_IRQ_NONE); >>> disable_irq(data->irq); >>> + return; >>> +clear_bits: >>> + /* Clear BUFF_EN, Clear END_EN, Clear MATCH_EN */ >>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07); >>> + >>> + /* Clear CEIR_EN */ >>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x00, 0x01); >>> + goto set_irqmask; >> >> I'm not convinced that adding a goto which goes backwards is making this >> code any more readible, just so that a local variable can be dropped. > > Thanks for your feedback. > > Is such a "backward jump" usual and finally required when you would like > to move a bit of common error handling code to the end without using extra > local variables and a few statements should still be performed after it? > I'm sorry, I can't parse this.