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 B7015C77B7C for ; Sat, 27 May 2023 11:13:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231180AbjE0LNe (ORCPT ); Sat, 27 May 2023 07:13:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54698 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229593AbjE0LNd (ORCPT ); Sat, 27 May 2023 07:13:33 -0400 Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 79725EC for ; Sat, 27 May 2023 04:13:30 -0700 (PDT) Received: by mail-wm1-x333.google.com with SMTP id 5b1f17b1804b1-3f611ccd06eso11079995e9.0 for ; Sat, 27 May 2023 04:13:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1685186009; x=1687778009; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=zNj5xIx+BSwromsYm9ia1o7bUGDaZEteHL4lXB89T5Q=; b=bd1q/GQAB8FX8qvHFUw+YjkqOlULq8xcqP2lcTWb1p58RYfjHUYsGKk3kk+oaotI2n lHZR+aB3Tp/ih7l/pGnYlyTx37Du23/EGbtzp2NyVof1zUBStBhCnDLnUkhFh4IdHK5c qXOoqNHi5SRIEIYsRVa83OZ8lm+ZrmhCCsivO5olA1MEfzoEJMw/zewg8YWkj+15j3ez p/x/iKgFvF21KqBV41QGtYbWzeL/J25PGnhxw9c3PLfzaD712YtRN1FpO16yiUL73Viz UW/6MUFOverOsVy2rmy2dM5+YhZJSfvMH1wolbtkrllGe0IApaKplSaN9smjhWDV4QLV Y4jQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685186009; x=1687778009; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zNj5xIx+BSwromsYm9ia1o7bUGDaZEteHL4lXB89T5Q=; b=R4PU9mUfhtpTDyyScmkSTk2aOdZpoaHXGe6qeqyjo8JtwfGwASJMh8CVzD6P64znEf gRUCTIoACDaD58ukK1tEX43z+GWMtHkFpaSubIzjmtNRizxyKW40js01fT0iZBFzxP+D vKo5wfR+OVYM3yGio42gpvYXGkOMBlKuKdxtQbErjVxtstHDjT7AQDc5zyEI9J9TYKtA zmZMcSKmOzFqHaTvEcGlvjFjKpaoOGYolePzf7ka1UBG9R2U0mEVw/itFrBxvbl6J+xK fYka+BLsfeFLW7huzoZY9v46pvVmEMZ+cGHtfhUJRnG9taWDWSmtvEvR4vn1GuQW+6CT IcZg== X-Gm-Message-State: AC+VfDzOvtdopAo2D7cLVDsKXuiCWo1HRQFBy52vT1XtRo8U6gJZmFHG z6p6UAivnfYyBCRQf5SJ+9O8gQ== X-Google-Smtp-Source: ACHHUZ656fhr7DfHYJLwVSn2SwLz4g3oSOi/Ftp4RjGh8gxHDAYhYorAxyhtTTTIga2lJwEcdRK2Ow== X-Received: by 2002:a5d:4911:0:b0:309:535c:c236 with SMTP id x17-20020a5d4911000000b00309535cc236mr3971606wrq.36.1685186008870; Sat, 27 May 2023 04:13:28 -0700 (PDT) Received: from [192.168.0.162] (188-141-3-169.dynamic.upc.ie. [188.141.3.169]) by smtp.gmail.com with ESMTPSA id v10-20020adfe28a000000b003063a1cdaf2sm7695793wri.48.2023.05.27.04.13.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 27 May 2023 04:13:28 -0700 (PDT) Message-ID: Date: Sat, 27 May 2023 12:13:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH v2 3/3] media: camss: Link CAMSS power domain Content-Language: en-US To: Yassine Oudjana Cc: Robert Foss , Todor Tomov , Andy Gross , Bjorn Andersson , Konrad Dybcio , Mauro Carvalho Chehab , Rob Herring , Krzysztof Kozlowski , Hans Verkuil , Yassine Oudjana , Vladimir Zapolskiy , linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org References: <20230526180712.8481-1-y.oudjana@protonmail.com> <20230526180712.8481-4-y.oudjana@protonmail.com> From: Bryan O'Donoghue In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On 27/05/2023 07:02, Yassine Oudjana wrote: >> Konrad pointed this out. >> >> Are you 100% sure you want to do this. We already have a way to count >> the # of power-domains in camss_configure_pd(). >> >> Your series is now adding a dependency on power-domain-names. >> >> Is there a good reason to add that dependency ? If not, then lets just >> take the code from camss_configure_pd() and make it so that it can be >> used/reused here. > > Is there a good reason not to?I found that using the existing > index-based method would unnecessarily complicate things since an extra > layer of checks would be needed to differentiate between MSM8996 and > TITAN SoCs, since those have the TITAN GDSC at the same index where the > CAMSS GDSC is now added for MSM8996. The same checks will also have to > be repeated in error paths and during cleanup. > > I guessed the only reason we were still using this method for the > existing PDs was to remain compatible with old DT as Konrad mentioned, > and since this CAMSS PD is only added now, I thought it'd be a good > opportunity to introduce power-domain-names and simplify things a bit. I think actually I agree with you but, I don't think you've gone far enough with this patch. Now that I look at this code a bit more, it looks like we need to place the TITAN/CAMSS GDSC last in the list of power-domains or the magic indices won't work. So my suggestion to you to place the CAMSS_GDSC in the power-domain list wouldn't work, unless it was the last entry,.. Having magic indices doesn't make much sense to me. Aside from anything else we don't document or require that indexing behavior in our Documentation. In fact, I'm wondering what is the use case of a vfe_lite on its own - without the TITAN_TOP GDSC switched on ? I'm looking at the block diagram of the clocks for the sm8250 the IFE_LITE is buried well inside of a series of other components.. The reverse OTOH holds. Full fat VFE can be collapsed individually, which is why they have their own GDSCs... OK, we should get away from magic indices ASAP. This is a good find, thank you for bringing it up. Could you take a named pointer for the CAMSS/TITAN instead of an index ? camss->genpd_camss_top * = camss->genpd_vfe[] = These have a very obvious meaning. We can read a top-level struct camss {} and immediately understand what is meant, whereas index = 0 doesn't mean anything and isn't obvious from the code anyway. 1. You're right we should introduce some kind of naming to break the bonds of magic indices. So lets do as you suggest and name the power-domains. However we should refactor the code to drop magic indices. 2. If and only if named power-domains are absent, fall back on legacy indexing. In this case we will assume legacy indexing assigns to our new named pointers. 3. New CAMSS dts will need to have named power-domains as a result. --- bod