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 D745BC7EE29 for ; Mon, 29 May 2023 23:33:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229698AbjE2XdG (ORCPT ); Mon, 29 May 2023 19:33:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38426 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229498AbjE2XdF (ORCPT ); Mon, 29 May 2023 19:33:05 -0400 Received: from fgw23-7.mail.saunalahti.fi (fgw23-7.mail.saunalahti.fi [62.142.5.84]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D9171BE for ; Mon, 29 May 2023 16:33:03 -0700 (PDT) Received: from localhost (88-113-26-95.elisa-laajakaista.fi [88.113.26.95]) by fgw23.mail.saunalahti.fi (Halon) with ESMTP id 2552c16e-fe79-11ed-b972-005056bdfda7; Tue, 30 May 2023 02:33:00 +0300 (EEST) From: andy.shevchenko@gmail.com Date: Tue, 30 May 2023 02:32:58 +0300 To: George Stark Cc: Andy Shevchenko , "jic23@kernel.org" , "lars@metafoo.de" , "neil.armstrong@linaro.org" , "khilman@baylibre.com" , "jbrunet@baylibre.com" , "martin.blumenstingl@googlemail.com" , "nuno.sa@analog.com" , "linux-iio@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-amlogic@lists.infradead.org" , kernel Subject: Re: [PATCH v2] meson saradc: add iio device attrib to switch channel 7 mux Message-ID: References: <20230527214854.126517-1-gnstark@sberdevices.ru> <9df82b13-7594-a8f0-f27e-87bce5a38b74@sberdevices.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <9df82b13-7594-a8f0-f27e-87bce5a38b74@sberdevices.ru> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mon, May 29, 2023 at 01:31:40AM +0300, George Stark kirjoitti: > On 5/28/23 13:46, Andy Shevchenko wrote: > > On Sun, May 28, 2023 at 12:48:54AM +0300, George Stark wrote: ... > > > + if (index >= ARRAY_SIZE(chan7_vol)) > > > + index = ARRAY_SIZE(chan7_vol) - 1; > > I think this is incorrect and prone to error in the future in case this array > > will be extended. What I would expect is to return something like "unknown". > > I agree this part is unclean. Actually the register's last 3 (out of 8) > possible values are stand for the same mux input "ch7_input". So > theoretically we can read from register a value out of array bounds. There > should be a comment at least. You can add there in the array to extend it up to 8 entries, so it will be clear. And for the rest you will return unknown / unsupported / etc. ... > > > +static const char * const chan7_vol[] = { > > > + "gnd", > > > + "vdd/4", > > > + "vdd/2", > > > + "vdd*3/4", > > > + "vdd", > > > + "ch7_input", > > > +}; > About the question of naming mux inputs from the other letter (vdd/2 vs > 0.5Vdd etc). > While I fully agree with you that point is better than slash but mixing > letter cases... should we? What's wrong with that? > e.g. this is how iio_info output looks like now: > ... >             voltage7:  (input) >             3 channel-specific attributes found: >                 attr  0: mean_raw value: 0 >                 attr  1: raw value: 1 >                 attr  2: scale value: 0.439453125 >         4 device-specific attributes found: >                 attr  0: chan7_mux value: gnd >                 attr  1: chan7_mux_available value: gnd vdd/4 vdd/2 vdd*3/4 > vdd ch7_input >                 attr  2: current_timestamp_clock value: realtime > >                 attr  3: waiting_for_supplier value: 0 > > or naming with Jonathan's approach > /sys/devices/platform/soc/fe000000.bus/fe002c00.adc/iio:device0/in_voltage_0.5vdd_raw See the alternative I proposed is to have _ delimiter. But that might make parsing a bit harder in user space. -- With Best Regards, Andy Shevchenko 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7AF80C7EE23 for ; Mon, 29 May 2023 23:33:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:Date:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=hsaXQZ5lB851ARFyy84AYZUF+9RF32PS/XplCCGkTGY=; b=kiuIYDpnucEbko r1+O9TFwltGYtzCVEoj36HP0tdPWp5uLXaKEoYVYziM1FiFyUW+jIAo4NW+QNYOWRBuQwLV96m2ry BIno39nL6YluL5szTG0pct0dxYHaI7wr0O2NMqqVgSJ+c8xBZRkujd6+sNt34ikgmVanw/hgghJ/c FxkJl2dYPWNeGxrY9gtFGXcTiECpN6Gf6K7BZ0zb0w+aGeRz874kq8uD1wzouDpbxUi8gJNRRfnEd 6Y3dE4FxXhtOo41TR6yp1rR5zMzTstlJv8UQ53PRzDCVlqd3sJU48Dm0LqOScqL9sZsnOolKK7CWk PjTatt1XNc+EWon2x52A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q3mMP-00BvQL-0O; Mon, 29 May 2023 23:33:09 +0000 Received: from fgw23-7.mail.saunalahti.fi ([62.142.5.84]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q3mMM-00BvP2-0k for linux-arm-kernel@lists.infradead.org; Mon, 29 May 2023 23:33:07 +0000 Received: from localhost (88-113-26-95.elisa-laajakaista.fi [88.113.26.95]) by fgw23.mail.saunalahti.fi (Halon) with ESMTP id 2552c16e-fe79-11ed-b972-005056bdfda7; Tue, 30 May 2023 02:33:00 +0300 (EEST) From: andy.shevchenko@gmail.com Date: Tue, 30 May 2023 02:32:58 +0300 To: George Stark Cc: Andy Shevchenko , "jic23@kernel.org" , "lars@metafoo.de" , "neil.armstrong@linaro.org" , "khilman@baylibre.com" , "jbrunet@baylibre.com" , "martin.blumenstingl@googlemail.com" , "nuno.sa@analog.com" , "linux-iio@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-amlogic@lists.infradead.org" , kernel Subject: Re: [PATCH v2] meson saradc: add iio device attrib to switch channel 7 mux Message-ID: References: <20230527214854.126517-1-gnstark@sberdevices.ru> <9df82b13-7594-a8f0-f27e-87bce5a38b74@sberdevices.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <9df82b13-7594-a8f0-f27e-87bce5a38b74@sberdevices.ru> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230529_163306_438115_BA33BCF5 X-CRM114-Status: GOOD ( 17.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Mon, May 29, 2023 at 01:31:40AM +0300, George Stark kirjoitti: > On 5/28/23 13:46, Andy Shevchenko wrote: > > On Sun, May 28, 2023 at 12:48:54AM +0300, George Stark wrote: ... > > > + if (index >=3D ARRAY_SIZE(chan7_vol)) > > > + index =3D ARRAY_SIZE(chan7_vol) - 1; > > I think this is incorrect and prone to error in the future in case this= array > > will be extended. What I would expect is to return something like "unkn= own". > = > I agree this part is unclean. Actually the register's last 3 (out of 8) > possible values are stand for the same mux input "ch7_input". So > theoretically we can read from register a value out of array bounds. There > should be a comment at least. You can add there in the array to extend it up to 8 entries, so it will be clear. And for the rest you will return unknown / unsupported / etc. ... > > > +static const char * const chan7_vol[] =3D { > > > + "gnd", > > > + "vdd/4", > > > + "vdd/2", > > > + "vdd*3/4", > > > + "vdd", > > > + "ch7_input", > > > +}; > About the question of naming mux inputs from the other letter (vdd/2 vs > 0.5Vdd etc). > While I fully agree with you that point is better than slash but mixing > letter cases... should we? What's wrong with that? > e.g. this is how iio_info output looks like now: > ... > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 voltage7:=A0 (input) > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 3 channel-specific attributes found: > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 0: mean_raw value: 0 > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 1: raw value: 1 > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 2: scale value: 0.4394531= 25 > =A0=A0 =A0=A0=A0=A0 4 device-specific attributes found: > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 0: chan7_mux value: gnd > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 1: chan7_mux_available va= lue: gnd vdd/4 vdd/2 vdd*3/4 > vdd ch7_input > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 2: current_timestamp_cloc= k value: realtime > = > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 3: waiting_for_supplier v= alue: 0 > = > or naming with Jonathan's approach > /sys/devices/platform/soc/fe000000.bus/fe002c00.adc/iio:device0/in_voltag= e_0.5vdd_raw See the alternative I proposed is to have _ delimiter. But that might make parsing a bit harder in user space. -- = With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E91BDC77B7E for ; Mon, 29 May 2023 23:33:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:Date:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mwoZtV81d5oZtrb11g18NVaqQx8JCG5RAZ663peGx/Y=; b=l3ISa21N06KmOe +YS9UeKtawWCtY3DRAhRpseIHdx8vsiqyGUPRlBjHsR8EOA81gmoAJogadXc0DnrmmG8vZ7AmzBZv 7R9YsdGXNV8gCo/2DCOMF7eZI5gHSwD+5sz0GyF6bm3c8vvwZf9n/r3eNEFzb+R7VlGlPyOrOEnZD 7js+xVZaPGreJehPgwmZMjbcpJIjc+mYH3+3wwuUNwK9eKzSwuX18BjGa8+Y6Xr1XxZ8d5nh4g9nN vcQ5pdTQT1LzSh0mrYX855mObNtSTq95SqACYEpn+7Sc7d3+xDQxOlBkvT2iTMROO7t3Gf9Ct0EPu By5EcBiqh69/PN0MaZ3g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q3mMP-00BvQt-23; Mon, 29 May 2023 23:33:09 +0000 Received: from fgw23-7.mail.saunalahti.fi ([62.142.5.84]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q3mMM-00BvP1-0k for linux-amlogic@lists.infradead.org; Mon, 29 May 2023 23:33:07 +0000 Received: from localhost (88-113-26-95.elisa-laajakaista.fi [88.113.26.95]) by fgw23.mail.saunalahti.fi (Halon) with ESMTP id 2552c16e-fe79-11ed-b972-005056bdfda7; Tue, 30 May 2023 02:33:00 +0300 (EEST) From: andy.shevchenko@gmail.com Date: Tue, 30 May 2023 02:32:58 +0300 To: George Stark Cc: Andy Shevchenko , "jic23@kernel.org" , "lars@metafoo.de" , "neil.armstrong@linaro.org" , "khilman@baylibre.com" , "jbrunet@baylibre.com" , "martin.blumenstingl@googlemail.com" , "nuno.sa@analog.com" , "linux-iio@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-amlogic@lists.infradead.org" , kernel Subject: Re: [PATCH v2] meson saradc: add iio device attrib to switch channel 7 mux Message-ID: References: <20230527214854.126517-1-gnstark@sberdevices.ru> <9df82b13-7594-a8f0-f27e-87bce5a38b74@sberdevices.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <9df82b13-7594-a8f0-f27e-87bce5a38b74@sberdevices.ru> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230529_163306_436537_027F4CD7 X-CRM114-Status: GOOD ( 16.72 ) X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org Mon, May 29, 2023 at 01:31:40AM +0300, George Stark kirjoitti: > On 5/28/23 13:46, Andy Shevchenko wrote: > > On Sun, May 28, 2023 at 12:48:54AM +0300, George Stark wrote: ... > > > + if (index >=3D ARRAY_SIZE(chan7_vol)) > > > + index =3D ARRAY_SIZE(chan7_vol) - 1; > > I think this is incorrect and prone to error in the future in case this= array > > will be extended. What I would expect is to return something like "unkn= own". > = > I agree this part is unclean. Actually the register's last 3 (out of 8) > possible values are stand for the same mux input "ch7_input". So > theoretically we can read from register a value out of array bounds. There > should be a comment at least. You can add there in the array to extend it up to 8 entries, so it will be clear. And for the rest you will return unknown / unsupported / etc. ... > > > +static const char * const chan7_vol[] =3D { > > > + "gnd", > > > + "vdd/4", > > > + "vdd/2", > > > + "vdd*3/4", > > > + "vdd", > > > + "ch7_input", > > > +}; > About the question of naming mux inputs from the other letter (vdd/2 vs > 0.5Vdd etc). > While I fully agree with you that point is better than slash but mixing > letter cases... should we? What's wrong with that? > e.g. this is how iio_info output looks like now: > ... > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 voltage7:=A0 (input) > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 3 channel-specific attributes found: > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 0: mean_raw value: 0 > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 1: raw value: 1 > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 2: scale value: 0.4394531= 25 > =A0=A0 =A0=A0=A0=A0 4 device-specific attributes found: > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 0: chan7_mux value: gnd > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 1: chan7_mux_available va= lue: gnd vdd/4 vdd/2 vdd*3/4 > vdd ch7_input > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 2: current_timestamp_cloc= k value: realtime > = > =A0=A0 =A0=A0=A0=A0 =A0=A0=A0 =A0=A0=A0 attr=A0 3: waiting_for_supplier v= alue: 0 > = > or naming with Jonathan's approach > /sys/devices/platform/soc/fe000000.bus/fe002c00.adc/iio:device0/in_voltag= e_0.5vdd_raw See the alternative I proposed is to have _ delimiter. But that might make parsing a bit harder in user space. -- = With Best Regards, Andy Shevchenko _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic