From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756210AbdKCX5q (ORCPT ); Fri, 3 Nov 2017 19:57:46 -0400 Received: from mail-pg0-f49.google.com ([74.125.83.49]:49398 "EHLO mail-pg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755846AbdKCX5o (ORCPT ); Fri, 3 Nov 2017 19:57:44 -0400 X-Google-Smtp-Source: ABhQp+SEHceUovurD0SHyV1Hct5DdeoPi1e0tmmuqk1yoQ3AqA5Q/eUHG6IvqV4vwQIDznDxVTvT1w== Date: Fri, 3 Nov 2017 16:57:40 -0700 From: Dmitry Torokhov To: Andi Shyti Cc: Rob Herring , linux-input@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Andi Shyti Subject: Re: [PATCH v5 2/2] Input: add support for the Samsung S6SY761 touchscreen Message-ID: <20171103235740.cmdbenzsqts2mtsy@dtor-ws> References: <20171102060726.3745-1-andi@etezian.org> <20171102060726.3745-3-andi@etezian.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171102060726.3745-3-andi@etezian.org> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andi, On Thu, Nov 02, 2017 at 03:07:26PM +0900, Andi Shyti wrote: > + > + err = s6sy761_hw_init(sdata); > + if (err) > + return err; > + > + sdata->input = devm_input_allocate_device(&client->dev); > + if (!sdata->input) > + return -ENOMEM; > + > + sdata->input->name = S6SY761_DEV_NAME; > + sdata->input->id.bustype = BUS_I2C; > + sdata->input->open = s6sy761_input_open; > + sdata->input->close = s6sy761_input_close; > + > + /* the range has been previously read in hw_init */ > + if (sdata->prop.max_x && sdata->prop.max_y) { You do not need make this a conditional, if sdata->prop.max_x or sdata->prop.max_y is 0 is it OK. However, there is a slight problem: you call hw_init() on resume, potentially updating prop.max_x and prop.max_y and taking them out of sync if they have been overridden by the DT data and touchscreen_parse_properties(). I think it would be best if s6sy761_hw_init() had 2 additional arguments for max_x and max_y and you passed them to input_set_abs_params() and left sdata->prop to be managed solely by touchscreen_parse_properties(). Also, is everything in s6sy761_hw_init() needed at resume time? Thanks. -- Dmitry