[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1b5b5bf0-43a5-2ec4-5570-891a710b85dd@linaro.org>
Date: Sat, 26 Aug 2023 13:05:25 +0100
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Konrad Dybcio <konrad.dybcio@...aro.org>, rfoss@...nel.org,
todor.too@...il.com, agross@...nel.org, andersson@...nel.org,
mchehab@...nel.org, hverkuil-cisco@...all.nl,
laurent.pinchart@...asonboard.com, sakari.ailus@...ux.intel.com,
andrey.konovalov@...aro.org
Cc: linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 10/15] media: qcom: camss: Allow clocks vfeN vfe_liteN
or vfe_lite
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@...aro.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
Powered by blists - more mailing lists