From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux Subject: Re: [PATCH 1/3] Input: ads7846 - introduce .gpio_pendown to get pendown state Date: Thu, 4 Sep 2008 11:00:51 +0100 Message-ID: <20080904100051.GC10426@flint.arm.linux.org.uk> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from caramon.arm.linux.org.uk ([78.32.30.218]:43073 "EHLO caramon.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752338AbYIDKBD (ORCPT ); Thu, 4 Sep 2008 06:01:03 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Eric Miao Cc: ARM Linux , linux-input@vger.kernel.org On Thu, Sep 04, 2008 at 05:21:05PM +0800, Eric Miao wrote: > The GPIO connected to ADS7846 nPENIRQ signal is usually used to get > the pendown state as well. Introduce a .gpio_pendown, and use this > to decide the pendown state if .get_pendown_state is NULL. > +static int get_pendown_state(struct ads7846 *ts) > +{ > + if (ts->get_pendown_state) > + return ts->get_pendown_state(); > + > + if (ts->gpio_pendown != -1) > + return !gpio_get_value(ts->gpio_pendown); > + > + return 0; > +} > + > + int gpio_pendown; /* the GPIO usually connected to PENIRQ > + * and used to decide pendown state > + * (-1) to use (*get_pendown_state) */ > + The comment doesn't match the code. get_pendown_state() is used if it's non-NULL, otherwise, the GPIO if it's not -1. I'm getting a little worried about these tests for valid gpio numbers in drivers - some check for non-zero, this one checks for not -1. I get the feeling that this is storing up problems for later. Maybe using gpio_is_valid() would be a good idea? But... a related question: do we need to do the check here? Surely either a pendown function is going to be supplied, or if not we're going to use a GPIO, in which case the GPIO better be valid. Maybe something to check at driver initialisation time only?