Received: by 2002:a05:7412:2a8c:b0:e2:908c:2ebd with SMTP id u12csp1027061rdh; Mon, 25 Sep 2023 00:26:09 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEJj6+PKl/XTFtGMlznlhkpszBCBQ+Mx3CiYDP8wDsD5FY97haP2HDyD15ct3CvXX73EU4u X-Received: by 2002:a05:6808:14d4:b0:3ae:237a:1338 with SMTP id f20-20020a05680814d400b003ae237a1338mr8556757oiw.1.1695626769546; Mon, 25 Sep 2023 00:26:09 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695626769; cv=none; d=google.com; s=arc-20160816; b=cdVyQwb40Q7BiUtCrOf8EYn3bjzTQ9QmMM1vgLtYE6E+JPDBx7QDoBTNbixJwvip1a 3T2bAmVrS2FhxXHYSEUSfHqxi0cUAW8kfyTPER5YwAI/G/oOSUoSeeizVLaumTfNB0cC jSuSBhyrgiaEaPXFZhnQBP3ymZOZbWJSZ6Bcn1yNoUnKeTt+EMduwXkteh2O81SrEE9E 7CCm4K+6d/fZ7eVO4t25BJ7gKlDFruWtwJWgxQ6QjAqMOZmXP1zzXP7w5KUpc/M5+HRU GhCtqlnGlUQptNX5XCsTSSzjMmiZcQE7qB/4zFkU028gHHER70QQTwyOia1tK9n1kwv2 WT/g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to:from :references:cc:to:content-language:subject:user-agent:mime-version :date:message-id; bh=VzXOeBAIcQ/tRmMrwAKCgHGiaU4dp9mgIVYXmKVhOYU=; fh=Y0PYaS9nrWl0DO0oYFMJ52zh3BA45I2L3hpwaCfZfdc=; b=cfQVdHH+fht5862Z8/MimwhxzfNmUw23/nvs3X4v4IGdfIcKsRGqfzelsySbdwYRZb mpKF4piQg1W9RlN/F3xeS22kZdn/vgYaPLKZCdlHIDQnokHd4PpXxhAHBBVyPhzFM7fs W5LIoZOPVFTER7j8D3/vB6coStr7WZS7yyIkcrKpKrwu5PsJVijmw2ctE0T5a+ucy8r0 I5jXAJp/7j7PoHjEIzAhU7a23fZVpuw7YyCfCg/RiWgq9tK7lFhgeImuS72MYc6PVGEx pVYpspY/XEgS6RBSwFG6aZbR5BVX0PxJAF64ttiAHIcJqxfCQ8cvq+qujyR5X5+ps3GN aPGg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=xs4all.nl Return-Path: Received: from agentk.vger.email (agentk.vger.email. [2620:137:e000::3:2]) by mx.google.com with ESMTPS id s16-20020a656450000000b00577448019cbsi9266886pgv.841.2023.09.25.00.26.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Sep 2023 00:26:09 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=xs4all.nl Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 948D98065823; Mon, 25 Sep 2023 00:12:12 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232363AbjIYHMJ (ORCPT + 99 others); Mon, 25 Sep 2023 03:12:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54236 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232462AbjIYHMC (ORCPT ); Mon, 25 Sep 2023 03:12:02 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 968C5124; Mon, 25 Sep 2023 00:11:55 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F1A7FC433C7; Mon, 25 Sep 2023 07:11:52 +0000 (UTC) Message-ID: <1e815d41-719a-4ca1-98e3-872f882ed03e@xs4all.nl> Date: Mon, 25 Sep 2023 09:11:52 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 17/17] media: qcom: camss: Comment CSID dt_id field Content-Language: en-US, nl To: Bryan O'Donoghue , rfoss@kernel.org, todor.too@gmail.com, agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org, mchehab@kernel.org, laurent.pinchart@ideasonboard.com, sakari.ailus@linux.intel.com, andrey.konovalov@linaro.org Cc: linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20230911131411.196033-1-bryan.odonoghue@linaro.org> <20230911131411.196033-18-bryan.odonoghue@linaro.org> From: Hans Verkuil In-Reply-To: <20230911131411.196033-18-bryan.odonoghue@linaro.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Mon, 25 Sep 2023 00:12:12 -0700 (PDT) On 11/09/2023 15:14, Bryan O'Donoghue wrote: > Digging into the documentation we find that the DT_ID bitfield is used to > map the six bit DT to a two bit ID code. This value is concatenated to the > VC bitfield to create a CID value. DT_ID is the two least significant bits > of CID and VC the most significant bits. > > Originally we set dt_id = vc * 4 in and then subsequently set dt_id = vc. > > commit 3c4ed72a16bc ("media: camss: sm8250: Virtual channels for CSID") > silently fixed the multiplication by four which would give a better > value for the generated CID without mentioning what was being done or why. > > Next up I haplessly changed the value back to "dt_id = vc * 4" since there > didn't appear to be any logic behind it. > > Hans asked what the change was for and I honestly couldn't remember the > provenance of it, so I dug in. > > Link: https://lore.kernel.org/linux-arm-msm/edd4bf9b-0e1b-883c-1a4d-50f4102c3924@xs4all.nl/ > > Add a comment so the next hapless programmer doesn't make this same > mistake. > > Signed-off-by: Bryan O'Donoghue > --- > drivers/media/platform/qcom/camss/camss-csid-gen2.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c > index 6ba2b10326444..cee50fc87e9de 100644 > --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c > +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c > @@ -352,6 +352,11 @@ static void __csid_configure_stream(struct csid_device *csid, u8 enable, u8 vc) > phy_sel = csid->phy.csiphy_id; > > if (enable) { > + /* > + * A value caled 'CID' gets generated internal to CAMSS logic caled -> called > + * which is a concatenation of [vc:6 | dt_id:2] hence we reuse vc:6? Do you mean bit 6 or do you mean the least significant 6 bits? Regardless, since the vc variable <= 3 (since MSM_CSID_MAX_SRC_STREAMS is 4), either interpretation makes little sense. And what does "DT" stand for? > + * the least significant two bits of the VC to 'stuff' the CID value. > + */ > u8 dt_id = vc; If dt_id should be the least significant two bits of vc, shouldn't this say: "= vc & 3;"? Or, alternatively, have a comment that vc <= 3? > > if (tg->enabled) { Regards, Hans