Received: by 2002:a05:6358:3188:b0:123:57c1:9b43 with SMTP id q8csp2806620rwd; Mon, 22 May 2023 04:48:33 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4IVcvRCp0mIkQPg2DXlCiM7Xg9+zF7tsM4NIehoVVHoqKc9JLfM/4jaihdxn2CAUdh9wAY X-Received: by 2002:a17:902:7008:b0:1ac:84dd:6d1f with SMTP id y8-20020a170902700800b001ac84dd6d1fmr10672024plk.1.1684756113165; Mon, 22 May 2023 04:48:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1684756113; cv=none; d=google.com; s=arc-20160816; b=scnUVgNH08OOpbJi7xjbIIKpxseiwMHihxM/XdXa58sLS9CUvhDflx/1/dhqbhGLw0 8MsFPdLlkd173lc0rcTjYoUHV1AZkYS3CvuwjwLV+OGmWdFAYp2ZNJ8H6rfCKsEg8EkL uq5ERc/dbeVS4HWdHYkEB9UFEMTMt1hCqlIoP/4R8hNUkGfe//k6nmnMoR+35ZNT0ZKJ ikPdpoNdeDusM2kisf992rDNTNOfAfZIlJbWttjIRMYQfWltSqrXM68cWOLJ41PcUo+V p7qs6HpcfUD3cCfAqJYlrVbm6gyiurwojUlxTHQsn/D+B81KXMH1N48/OLVAXwaHHUkd MBkA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:content-transfer-encoding :references:in-reply-to:date:cc:to:from:subject:message-id :dkim-signature; bh=0n8l4247LeEWaLKQ0FnSfNM9KwjF8gdrQk1OZZ4VON0=; b=jz6XdtjvVEw2j1sgZBPAoHjh1NF+VGQNMMsxbw4XXsWnfxdyZbBhnED5Yy1RriR/FF oZ9v1K9RQHG/+gcBKOQ8nFx8eUptJa9jSaFDXoNJc5CJ8t6Jw3H7MGDWp4wYSlGwJuP1 DfD2LtUeW4AzrxKPLDVU1HUFZHQpul7h8Oq4ADtfUUd7ACBT/pgHEq/TanabCjhckPP/ hXhekKUZwdf36qVpptW6GEIEas2Yb2xM1tmNjjPbNb3zUqp3PW+syebJ0YZPLxT7O8vh RU+myVwdnEwd0zIugKyzgjvpCrWk1nAYOzdVSyNYYFZGUBPNDBY5nGSci5lkTkN6MWDH YTXQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@crapouillou.net header.s=mail header.b=vgcIeJ3S; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=crapouillou.net Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id lg13-20020a170902fb8d00b001ab0727a2c0si4321364plb.424.2023.05.22.04.48.18; Mon, 22 May 2023 04:48:33 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@crapouillou.net header.s=mail header.b=vgcIeJ3S; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=crapouillou.net Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233310AbjEVLgW (ORCPT + 99 others); Mon, 22 May 2023 07:36:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49284 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233157AbjEVLgU (ORCPT ); Mon, 22 May 2023 07:36:20 -0400 Received: from aposti.net (aposti.net [89.234.176.197]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8C946F1; Mon, 22 May 2023 04:36:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=crapouillou.net; s=mail; t=1684755358; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0n8l4247LeEWaLKQ0FnSfNM9KwjF8gdrQk1OZZ4VON0=; b=vgcIeJ3SYVpE+jcU3BpOIfJQV6mo8nqBt41pXsNvzViL3qMlZQ0sZPJ1saiDlTQfuT4dN9 RK6OVIflDrt5aZua0j4XeZJTSAF7Eqb1Beyly8LBtIrwXdbaNCT/N3dcJZPxFAV50yQ8n/ gVjLZTufknvT5paH8YMPKdkWWVXlbQc= Message-ID: <6b88623e44b2a98a2e5d8d6d2453f92eb1b673ae.camel@crapouillou.net> Subject: Re: [PATCH v2 1/2] iio/adc: ingenic: Fix channel offsets in buffer From: Paul Cercueil To: Andy Shevchenko Cc: Artur Rojek , Jonathan Cameron , Dmitry Torokhov , Chris Morgan , linux-mips@vger.kernel.org, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-input@vger.kernel.org Date: Mon, 22 May 2023 13:35:56 +0200 In-Reply-To: References: <20230521225901.388455-1-contact@artur-rojek.eu> <20230521225901.388455-2-contact@artur-rojek.eu> <2fc0874ce8a802aeb98e553b15e27fb4d4b75a1c.camel@crapouillou.net> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_PASS,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andy, Le lundi 22 mai 2023 =C3=A0 14:05 +0300, Andy Shevchenko a =C3=A9crit=C2=A0= : > On Mon, May 22, 2023 at 1:23=E2=80=AFPM Paul Cercueil > wrote: > > Le lundi 22 mai 2023 =C3=A0 13:18 +0300, Andy Shevchenko a =C3=A9crit : > > > On Mon, May 22, 2023 at 1:15=E2=80=AFPM Andy Shevchenko > > > wrote: > > > > On Mon, May 22, 2023 at 1:59=E2=80=AFAM Artur Rojek > > > > wrote: >=20 > ... >=20 > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 u16 tdat[6]; > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 u32 val; > > > > > + > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 memset(tdat, 0, ARRAY_SIZE(= tdat)); > > > >=20 > > > > Yeah, as LKP tells us this should be sizeof() instead of > > > > ARRAY_SIZE(). > > > >=20 > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 for (i =3D 0; mask && i < A= RRAY_SIZE(tdat); mask >>=3D 2) > > > > > { > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0 if (mask & 0x3) { > > > >=20 > > > > (for the consistency it has to be GENMASK(), but see below) > > > >=20 > > > > First of all, strictly speaking we should use the full mask > > > > without > > > > limiting it to the 0 element in the array (I'm talking about > > > > active_scan_mask). > > > >=20 > > > > That said, we may actually use bit operations here in a better > > > > way, > > > > i.e. > > > >=20 > > > > =C2=A0 unsigned long mask =3D active_scan_mask[0] & > > > > (active_scan_mask[0] - > > > > 1); > > > >=20 > > > > =C2=A0 j =3D 0; > > > > =C2=A0 for_each_set_bit(i, active_scan_mask, ...) { > > > > =C2=A0=C2=A0=C2=A0 val =3D readl(...); > > > > =C2=A0=C2=A0=C2=A0 /* Two channels per sample. Demux active. */ > > > > =C2=A0=C2=A0=C2=A0 tdat[j++] =3D val >> (16 * (i % 2)); > > >=20 > > > Alternatively > > >=20 > > > =C2=A0=C2=A0=C2=A0=C2=A0 /* Two channels per sample. Demux active. */ > > > =C2=A0=C2=A0=C2=A0=C2=A0 if (i % 2) > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tdat[j++] =3D upper_16_bits(val)= ; > > > =C2=A0=C2=A0=C2=A0=C2=A0 else > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tdat[j++] =3D lower_16_bits(val)= ; > > >=20 > > > which may be better to read. > >=20 > > It's not if/else though. You would check (i % 2) for the upper 16 > > bits, > > and (i % 1) for the lower 16 bits. Both can be valid at the same > > time. >=20 > Are you sure? Have you looked into the proposed code carefully? Yes. I co-wrote the original code, I know what it's supposed to do. >=20 > What probably can be done differently is the read part, that can be > called once. But looking at it I'm not sure how it's supposed to work > at all, since the address is always the same. How does the code and > hardware are in sync with the channels? It's a FIFO. Cheers, -Paul >=20 > > > > =C2=A0 } > > > >=20 > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 val = =3D readl(adc->base + > > > > > JZ_ADC_REG_ADTCH); > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Tw= o channels per sample. Demux > > > > > active. > > > > > */ > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (m= ask & BIT(0)) > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tdat[i++] =3D val & 0xffff; > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (m= ask & BIT(1)) > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tdat[i++] =3D val >> 16; > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0 } > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >=20