Message ID | 20230823104444.1954663-11-bryan.odonoghue@linaro.org (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Hans Verkuil |
Headers |
Received: from vger.kernel.org ([23.128.96.18]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from <linux-media-owner@vger.kernel.org>) id 1qYlMX-006DWN-T6; Wed, 23 Aug 2023 10:45:22 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234077AbjHWKpT (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Wed, 23 Aug 2023 06:45:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58156 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234032AbjHWKpJ (ORCPT <rfc822;linux-media@vger.kernel.org>); Wed, 23 Aug 2023 06:45:09 -0400 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9DBE1E7B for <linux-media@vger.kernel.org>; Wed, 23 Aug 2023 03:45:03 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id ffacd0b85a97d-31771bb4869so4897157f8f.0 for <linux-media@vger.kernel.org>; Wed, 23 Aug 2023 03:45:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1692787502; x=1693392302; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=rHxvfmBq2wQcZEqJeTw0BxrGFmAlBN3kn0Ir0lpbaw4=; b=RK7fSEqchuMcLnME21sx1cAxKm1ohh4rOeH0kFTbeYZNLLOc0KqB2kE/OSFxWbVYlI WQ6FZyX/UIxy+szLwqxEuP7a+1lfwfoMpT8LXtRFBwtkb5rprWjdX+f4ZOf2h6dM1imB 5KkUanGpvDxKsV6PY+EM+bYuylgTH48zWM+yC0afo8XJUqRHMq1HWs8K0BzXHLJVVzwF B3Y5QvsiD/JXzmGYBTpBEvXKWAh/b8FPLx3sisw3JIVtJ8wKv/B1lBhskFc71M+HUWzX qBiVGJvx5m6GXcXvBU17GbfPZQb6YJMkMjLZq8XZiks9Vb1aRNSUrHdCFH2oORQsT214 hg1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692787502; x=1693392302; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rHxvfmBq2wQcZEqJeTw0BxrGFmAlBN3kn0Ir0lpbaw4=; b=dJXbFEDcMp9f1CG4OGgSQoaEaD4tLWcFIuuLUV0MQjPcF6XAXk+c0lmFWiZdtlbgMF bRfwxdYEeSSDRRqiqOWufYYY6YrYhOuH1xPP4oAvyzfTcss3dg22Bj8b35TIfwmFkqHh TK3zeswQwryCV7W9sTD873umDD2jGp38gJaVO8AH2N1yhJd1HLOpT3W//UGPmYtFWnb4 7d5LOObE9TkM0EmIjNNbbUJZs/0DMIetrufo0SjsTW/z9k1OyjAIB1Rqdms5vGMBi21e oXoGA/Ffa6a6y8u5j+lRXCy5DnJKJQ7IYVvUO7NQDLneHsShkojYawMjHR7oTwvsGigc YwRQ== X-Gm-Message-State: AOJu0YzhRdugnlQW+/YZjHbqRSgjHISPEcRmwXETeGSrTThjpvODrCLh LTojDZOgjq/cA4MJ796dWl29xLZ4yq8Hp+jSjKk= X-Google-Smtp-Source: AGHT+IFMOCh8NcCdKiEJDomH2o9MH+X6gGMRhmLARi6n1ID7VDWqrST5NLVd3DdYr49+F4PTrMtOiw== X-Received: by 2002:adf:dd90:0:b0:315:9e1b:4ea6 with SMTP id x16-20020adfdd90000000b003159e1b4ea6mr9105973wrl.58.1692787502156; Wed, 23 Aug 2023 03:45:02 -0700 (PDT) Received: from sagittarius-a.chello.ie (188-141-3-169.dynamic.upc.ie. [188.141.3.169]) by smtp.gmail.com with ESMTPSA id n4-20020a5d4204000000b0031c5dda3aedsm6281213wrq.95.2023.08.23.03.45.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Aug 2023 03:45:01 -0700 (PDT) From: Bryan O'Donoghue <bryan.odonoghue@linaro.org> To: rfoss@kernel.org, todor.too@gmail.com, bryan.odonoghue@linaro.org, agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org, mchehab@kernel.org, hverkuil-cisco@xs4all.nl, 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 Subject: [PATCH v3 10/15] media: qcom: camss: Allow clocks vfeN vfe_liteN or vfe_lite Date: Wed, 23 Aug 2023 11:44:39 +0100 Message-ID: <20230823104444.1954663-11-bryan.odonoghue@linaro.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230823104444.1954663-1-bryan.odonoghue@linaro.org> References: <20230823104444.1954663-1-bryan.odonoghue@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-LSpam-Score: -2.5 (--) X-LSpam-Report: No, score=-2.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1 autolearn=ham autolearn_force=no |
Series |
media: qcom: camss: Add parameter passing to remove several outstanding bugs
|
|
Commit Message
Bryan O'Donoghue
Aug. 23, 2023, 10:44 a.m. UTC
The number of Video Front End - VFE or Image Front End - IFE supported
with new SoCs can vary both for the full and lite cases.
For example sdm845 has one vfe_lite and two vfe interfaces with the vfe
clock called simply "vfe_lite" with no integer postfix. sc8280xp has four
vfe and four vfe lite blocks.
We need to support the following clock name formats
- vfeN
- vfe_liteN
- vfe_lite
with N being any reasonably sized integer.
There are two sites in this code which need to do the same thing,
constructing and matching strings with the pattern above, so encapsulate
the logic in one function.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/media/platform/qcom/camss/camss-vfe.c | 22 ++++++++++++++-----
1 file changed, 16 insertions(+), 6 deletions(-)
Comments
On 23.08.2023 12:44, Bryan O'Donoghue wrote: > The number of Video Front End - VFE or Image Front End - IFE supported > with new SoCs can vary both for the full and lite cases. > > For example sdm845 has one vfe_lite and two vfe interfaces with the vfe > clock called simply "vfe_lite" with no integer postfix. sc8280xp has four > vfe and four vfe lite blocks. > > We need to support the following clock name formats > > - vfeN > - vfe_liteN > - vfe_lite > > with N being any reasonably sized integer. > > There are two sites in this code which need to do the same thing, > constructing and matching strings with the pattern above, so encapsulate > the logic in one function. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/media/platform/qcom/camss/camss-vfe.c | 22 ++++++++++++++----- > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c > index 8f48401e31cd3..73380e75dbb22 100644 > --- a/drivers/media/platform/qcom/camss/camss-vfe.c > +++ b/drivers/media/platform/qcom/camss/camss-vfe.c > @@ -437,6 +437,20 @@ void vfe_isr_reset_ack(struct vfe_device *vfe) > complete(&vfe->reset_complete); > } > > +static int vfe_match_clock_names(struct vfe_device *vfe, > + struct camss_clock *clock) > +{ > + char vfe_name[CAMSS_RES_MAX]; > + char vfe_lite_name[CAMSS_RES_MAX]; I don't think using the "number of resources" define to define the maximum length of a resource name is a good idea. Perhaps we can do: char vfe_name[5]; /* "vfeX\0" */ char vfe_lite_name[10]; /* "vfe_liteX\0" */ if index > 9 return INCREASE_THE_BUFFER Konrad
On 26/08/2023 11:08, Konrad Dybcio wrote: > On 23.08.2023 12:44, Bryan O'Donoghue wrote: >> The number of Video Front End - VFE or Image Front End - IFE supported >> with new SoCs can vary both for the full and lite cases. >> >> For example sdm845 has one vfe_lite and two vfe interfaces with the vfe >> clock called simply "vfe_lite" with no integer postfix. sc8280xp has four >> vfe and four vfe lite blocks. >> >> We need to support the following clock name formats >> >> - vfeN >> - vfe_liteN >> - vfe_lite >> >> with N being any reasonably sized integer. >> >> There are two sites in this code which need to do the same thing, >> constructing and matching strings with the pattern above, so encapsulate >> the logic in one function. >> >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> --- >> drivers/media/platform/qcom/camss/camss-vfe.c | 22 ++++++++++++++----- >> 1 file changed, 16 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c >> index 8f48401e31cd3..73380e75dbb22 100644 >> --- a/drivers/media/platform/qcom/camss/camss-vfe.c >> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c >> @@ -437,6 +437,20 @@ void vfe_isr_reset_ack(struct vfe_device *vfe) >> complete(&vfe->reset_complete); >> } >> >> +static int vfe_match_clock_names(struct vfe_device *vfe, >> + struct camss_clock *clock) >> +{ >> + char vfe_name[CAMSS_RES_MAX]; >> + char vfe_lite_name[CAMSS_RES_MAX]; > I don't think using the "number of resources" define to define > the maximum length of a resource name is a good idea. > > Perhaps we can do: > > char vfe_name[5]; /* "vfeX\0" */ > char vfe_lite_name[10]; /* "vfe_liteX\0" */ > > if index > 9 > return INCREASE_THE_BUFFER > > Konrad I'm reluctant to fix only the VFE clock name string length in isolation, plus I'm aware of another patchset coming down the line from other people which will likely address the string length stuff. But in the interests of consensus I will restrict the length in the helper. --- bod
On Wed, Aug 23, 2023 at 11:44:39AM +0100, Bryan O'Donoghue wrote: > The number of Video Front End - VFE or Image Front End - IFE supported > with new SoCs can vary both for the full and lite cases. > > For example sdm845 has one vfe_lite and two vfe interfaces with the vfe > clock called simply "vfe_lite" with no integer postfix. sc8280xp has four > vfe and four vfe lite blocks. > > We need to support the following clock name formats > > - vfeN > - vfe_liteN > - vfe_lite > > with N being any reasonably sized integer. > > There are two sites in this code which need to do the same thing, > constructing and matching strings with the pattern above, so encapsulate > the logic in one function. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/media/platform/qcom/camss/camss-vfe.c | 22 ++++++++++++++----- > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c > index 8f48401e31cd3..73380e75dbb22 100644 > --- a/drivers/media/platform/qcom/camss/camss-vfe.c > +++ b/drivers/media/platform/qcom/camss/camss-vfe.c > @@ -437,6 +437,20 @@ void vfe_isr_reset_ack(struct vfe_device *vfe) > complete(&vfe->reset_complete); > } > > +static int vfe_match_clock_names(struct vfe_device *vfe, > + struct camss_clock *clock) > +{ > + char vfe_name[CAMSS_RES_MAX]; > + char vfe_lite_name[CAMSS_RES_MAX]; > + > + snprintf(vfe_name, sizeof(vfe_name), "vfe%d", vfe->id); > + snprintf(vfe_lite_name, sizeof(vfe_lite_name), "vfe_lite%d", vfe->id); > + > + return (!strcmp(clock->name, vfe_name) || > + !strcmp(clock->name, vfe_lite_name) || > + !strcmp(clock->name, "vfe_lite")); The code below will match "vfe0", "vfe1" and "vfe_lite", which the code here matches "vfe%d", "vfe_life%d" and "vfe_lite". The commit message doesn't explain why you can restrict the clock name to "vfe%d" instead of matching both "vfe0" and "vfe1". I could try to guess, but it's better to clarify it in the commit message. > +} > + > /* > * vfe_set_clock_rates - Calculate and set clock rates on VFE module > * @vfe: VFE device > @@ -460,9 +474,7 @@ static int vfe_set_clock_rates(struct vfe_device *vfe) > for (i = 0; i < vfe->nclocks; i++) { > struct camss_clock *clock = &vfe->clock[i]; > > - if (!strcmp(clock->name, "vfe0") || > - !strcmp(clock->name, "vfe1") || > - !strcmp(clock->name, "vfe_lite")) { > + if (vfe_match_clock_names(vfe, clock)) { > u64 min_rate = 0; > long rate; > > @@ -543,9 +555,7 @@ static int vfe_check_clock_rates(struct vfe_device *vfe) > for (i = 0; i < vfe->nclocks; i++) { > struct camss_clock *clock = &vfe->clock[i]; > > - if (!strcmp(clock->name, "vfe0") || > - !strcmp(clock->name, "vfe1") || > - !strcmp(clock->name, "vfe_lite")) { > + if (vfe_match_clock_names(vfe, clock)) { > u64 min_rate = 0; > unsigned long rate; >
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c index 8f48401e31cd3..73380e75dbb22 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe.c +++ b/drivers/media/platform/qcom/camss/camss-vfe.c @@ -437,6 +437,20 @@ void vfe_isr_reset_ack(struct vfe_device *vfe) complete(&vfe->reset_complete); } +static int vfe_match_clock_names(struct vfe_device *vfe, + struct camss_clock *clock) +{ + char vfe_name[CAMSS_RES_MAX]; + char vfe_lite_name[CAMSS_RES_MAX]; + + snprintf(vfe_name, sizeof(vfe_name), "vfe%d", vfe->id); + snprintf(vfe_lite_name, sizeof(vfe_lite_name), "vfe_lite%d", vfe->id); + + return (!strcmp(clock->name, vfe_name) || + !strcmp(clock->name, vfe_lite_name) || + !strcmp(clock->name, "vfe_lite")); +} + /* * vfe_set_clock_rates - Calculate and set clock rates on VFE module * @vfe: VFE device @@ -460,9 +474,7 @@ static int vfe_set_clock_rates(struct vfe_device *vfe) for (i = 0; i < vfe->nclocks; i++) { struct camss_clock *clock = &vfe->clock[i]; - if (!strcmp(clock->name, "vfe0") || - !strcmp(clock->name, "vfe1") || - !strcmp(clock->name, "vfe_lite")) { + if (vfe_match_clock_names(vfe, clock)) { u64 min_rate = 0; long rate; @@ -543,9 +555,7 @@ static int vfe_check_clock_rates(struct vfe_device *vfe) for (i = 0; i < vfe->nclocks; i++) { struct camss_clock *clock = &vfe->clock[i]; - if (!strcmp(clock->name, "vfe0") || - !strcmp(clock->name, "vfe1") || - !strcmp(clock->name, "vfe_lite")) { + if (vfe_match_clock_names(vfe, clock)) { u64 min_rate = 0; unsigned long rate;