From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C9936C25B07 for ; Thu, 11 Aug 2022 00:59:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232777AbiHKA67 (ORCPT ); Wed, 10 Aug 2022 20:58:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39092 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229488AbiHKA6y (ORCPT ); Wed, 10 Aug 2022 20:58:54 -0400 Received: from mail-pg1-x530.google.com (mail-pg1-x530.google.com [IPv6:2607:f8b0:4864:20::530]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AF1BB647DE; Wed, 10 Aug 2022 17:58:53 -0700 (PDT) Received: by mail-pg1-x530.google.com with SMTP id f11so15821569pgj.7; Wed, 10 Aug 2022 17:58:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc; bh=k0bhUMiU7HDf8ByyUWn8v9bASxgxCe2ocaeUGV4VFSk=; b=CJfZY3aExNLiELPRL69Xbai+jTrHNRWlkO3/tiVRZWuDzVwkRQ9icJmwwirqfCJksD Uorpvrn/drkM17+vZj/ZE0Rpr61dZdPRtY3Rv1vhdjWWHnOsy9hs3Bgp//huYFykVXcW Qx7DzH64XhoRqq6sz0PzfWnTTMZo7c9GXlZm6orBQhBWEjprEyAVLNZdeNkhrXg7olWw aBzwy5ejz1OlcK3Z/XBNF6Ajh7+ls4ApuTnpcHEEAFs1iqJdruNFaB+Ns+b/4C4X+6YO QGttH8bMn/g4/Kk8pZ1GbTiPyWg+H8cBn3EEzYSzMUB6OxbyVa0A33nrTMS2wnc0mRiF Wa6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc; bh=k0bhUMiU7HDf8ByyUWn8v9bASxgxCe2ocaeUGV4VFSk=; b=Ja1iQ6N5VhO+VDAxyjZI6ByneycOPIkgl7yas6N1RW4ALOG/QU+A9yoz85YJCsCwXh fwWoNdzwsJFeP3oNFZSl0L/7eZtD78BWq5TNOausEOkrDq+coCMT4J1w7eOlnWC/rIrq ugSY/ApcW49i+EIiitgwe3/siKDhi8mb5wypnpOvbakxNvium8u7QvPIz8NCbWtCHWtP NwMwOC7Yr1XzviyaqrjT5t174XDp1HFZnwDLqX391qxVSNoKfc37Dmo1HqIYWLZGoR8s uxYyIdG6tMlVWpM3qTyAm78epteHY6XqPG6ALYRi0x96JDWdqcrvDrQV/p4TBfkdU6SS 15nA== X-Gm-Message-State: ACgBeo2bhSyNpQb0oBnzNG1EUg3boDB1iNR98wrX3EGVlAFpcf7mrNCL AUSGLgj0KKgjAO4ZUbRv5hM= X-Google-Smtp-Source: AA6agR70H85HGA3vVInnbpSwUrfmwyci7JmiXRzPSUpGbugeNcYRT+UHskzbJLTRUwIyGzGtchgB/A== X-Received: by 2002:a63:ff19:0:b0:41a:8f88:5703 with SMTP id k25-20020a63ff19000000b0041a8f885703mr25186834pgi.355.1660179532829; Wed, 10 Aug 2022 17:58:52 -0700 (PDT) Received: from google.com ([2620:15c:202:201:a3c8:d6b9:a5c2:1eca]) by smtp.gmail.com with ESMTPSA id l17-20020a170902f69100b0016a091eb88esm13674287plg.126.2022.08.10.17.58.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Aug 2022 17:58:51 -0700 (PDT) Date: Wed, 10 Aug 2022 17:58:48 -0700 From: Dmitry Torokhov To: Artur Rojek Cc: Jonathan Cameron , Chris Morgan , linux-input@vger.kernel.org, linux-iio@vger.kernel.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, heiko@sntech.de, paul@crapouillou.net, Chris Morgan , Maya Matuszczyk Subject: Re: [PATCH v12 2/3] Input: adc-joystick - Add polled input device support Message-ID: References: <20220805171016.21217-1-macroalpha82@gmail.com> <20220805171016.21217-3-macroalpha82@gmail.com> <20220806152042.39bc5351@jic23-huawei> <9399f54366be973dba36a70cb3dcbfd9@artur-rojek.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9399f54366be973dba36a70cb3dcbfd9@artur-rojek.eu> Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org On Sat, Aug 06, 2022 at 04:19:21PM +0200, Artur Rojek wrote: > On 2022-08-06 16:20, Jonathan Cameron wrote: > > On Fri, 5 Aug 2022 12:10:15 -0500 > > Chris Morgan wrote: > > > > > From: Chris Morgan > > > > > > Add polled input device support to the adc-joystick driver. This is > > > useful for devices which do not have hardware capable triggers on > > > their SARADC. Code modified from adc-joystick.c changes made by Maya > > > Matuszczyk. > > > > > > Signed-off-by: Maya Matuszczyk > > > Signed-off-by: Chris Morgan > > Hi Chris, > > > > Trying to avoid too much indentation has lead to an odd code structure. > > Still minor thing, so either way this looks fine to me. > > > > Reviewed-by: Jonathan Cameron > > > > > --- > > > drivers/input/joystick/adc-joystick.c | 44 > > > +++++++++++++++++++++++++-- > > > 1 file changed, 41 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/input/joystick/adc-joystick.c > > > b/drivers/input/joystick/adc-joystick.c > > > index 78ebca7d400a..77dfb7dd96eb 100644 > > > --- a/drivers/input/joystick/adc-joystick.c > > > +++ b/drivers/input/joystick/adc-joystick.c > > > @@ -26,8 +26,23 @@ struct adc_joystick { > > > struct adc_joystick_axis *axes; > > > struct iio_channel *chans; > > > int num_chans; > > > + bool polled; > > > }; > > > > > > +static void adc_joystick_poll(struct input_dev *input) > > > +{ > > > + struct adc_joystick *joy = input_get_drvdata(input); > > > + int i, val, ret; > > > + > > > + for (i = 0; i < joy->num_chans; i++) { > > > + ret = iio_read_channel_raw(&joy->chans[i], &val); > > > + if (ret < 0) > > > + return; > > > + input_report_abs(input, joy->axes[i].code, val); > > > + } > > > + input_sync(input); > > > +} > > > + > > > static int adc_joystick_handle(const void *data, void *private) > > > { > > > struct adc_joystick *joy = private; > > > @@ -179,6 +194,7 @@ static int adc_joystick_probe(struct > > > platform_device *pdev) > > > int error; > > > int bits; > > > int i; > > > + unsigned int poll_interval; > > > > > > joy = devm_kzalloc(dev, sizeof(*joy), GFP_KERNEL); > > > if (!joy) > > > @@ -192,8 +208,21 @@ static int adc_joystick_probe(struct > > > platform_device *pdev) > > > return error; > > > } > > > > > > - /* Count how many channels we got. NULL terminated. */ > > > + if (device_property_present(dev, "poll-interval")) { > > > + error = device_property_read_u32(dev, "poll-interval", > > > + &poll_interval); > > > + if (error) > > > + return error; > > > + joy->polled = true; device_property_read_u32() return -EINVAL if property is not present, so we can write: error = device_property_read_u32(dev, "poll-interval", &poll_interval); if (error) { /* -EINVAL means the property is absent. */ if (error != -EINVAL) return error; } else if (poll_interval == 0) { dev_err(...); return -EINVAL; } else { joy->polled = true; } > > > + } > > > + > > > + /* > > > + * Count how many channels we got. NULL terminated. > > > + * Do not check the storage size if using polling. > > > + */ > > > for (i = 0; joy->chans[i].indio_dev; i++) { > > > + if (joy->polled) > > > + continue; > > > > Whilst I can see why did this, it is a rather 'unusual' code structure > > and that makes me a tiny bit uncomfortable. However if everyone else > > is happy with this then fair enough (I see it was Artur's suggestion to > > handle it like this). > Yep, I'm fine with the way it is right now :) > > Acked-by: Artur Rojek > > > > > > bits = joy->chans[i].channel->scan_type.storagebits; > > > if (!bits || bits > 16) { > > > dev_err(dev, "Unsupported channel storage size\n"); > > > @@ -215,8 +244,14 @@ static int adc_joystick_probe(struct > > > platform_device *pdev) > > > joy->input = input; > > > input->name = pdev->name; > > > input->id.bustype = BUS_HOST; > > > - input->open = adc_joystick_open; > > > - input->close = adc_joystick_close; > > > + > > > + if (joy->polled) { > > > + input_setup_polling(input, adc_joystick_poll); > > > + input_set_poll_interval(input, poll_interval); > > > + } else { > > > + input->open = adc_joystick_open; > > > + input->close = adc_joystick_close; > > > + } > > > > > > error = adc_joystick_set_axes(dev, joy); > > > if (error) > > > @@ -229,6 +264,9 @@ static int adc_joystick_probe(struct > > > platform_device *pdev) > > > return error; > > > } > > > > > > + if (joy->polled) > > > + return 0; > > > + This is no longer compatible with the latest driver code as input device registration has been moved to the very end, so you actually need to move getting bugger and setting up cleanup action into the "else" clause of "if (joy->polled)", even though it adds indentation level. > > > joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy); > > > if (IS_ERR(joy->buffer)) { > > > dev_err(dev, "Unable to allocate callback buffer\n"); Thanks. -- Dmitry