| Message ID | 20230911131411.196033-12-bryan.odonoghue@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: qcom: camss: Add parameter passing to remove several outstanding bugs | expand |
On 11/09/2023 15:14, 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. > > At the moment we declare vfe_lite0 and vfe_lite1 for sm8250 but never set > those clocks because we don't match the strings. > > 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 db8f68819ded9..f3cf387e4907e 100644 > --- a/drivers/media/platform/qcom/camss/camss-vfe.c > +++ b/drivers/media/platform/qcom/camss/camss-vfe.c > @@ -431,6 +431,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[6]; /* vfeXX\0 */ > + char vfe_lite_name[11]; /* vfe_liteXX\0 */ > + > + 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")); > +} I'm getting this compiler warning: drivers/media/platform/qcom/camss/camss-vfe.c: In function 'vfe_match_clock_names': drivers/media/platform/qcom/camss/camss-vfe.c:483:52: warning: 'snprintf' output may be truncated before the last format character [-Wformat-truncation=] 483 | snprintf(vfe_name, sizeof(vfe_name), "vfe%d", vfe->id); | ^ Since vfe->id is a u8 I would just increase both the vfe_name and vfe_lite_name sizes by 1. Regards, Hans > + > /* > * vfe_set_clock_rates - Calculate and set clock rates on VFE module > * @vfe: VFE device > @@ -454,9 +468,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; > > @@ -537,9 +549,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; >
On 25/09/2023 08:10, Hans Verkuil wrote: > On 11/09/2023 15:14, 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. >> >> At the moment we declare vfe_lite0 and vfe_lite1 for sm8250 but never set >> those clocks because we don't match the strings. >> >> 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 db8f68819ded9..f3cf387e4907e 100644 >> --- a/drivers/media/platform/qcom/camss/camss-vfe.c >> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c >> @@ -431,6 +431,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[6]; /* vfeXX\0 */ >> + char vfe_lite_name[11]; /* vfe_liteXX\0 */ >> + >> + 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")); >> +} > > I'm getting this compiler warning: > > drivers/media/platform/qcom/camss/camss-vfe.c: In function 'vfe_match_clock_names': > drivers/media/platform/qcom/camss/camss-vfe.c:483:52: warning: 'snprintf' output may be truncated before the last format character [-Wformat-truncation=] > 483 | snprintf(vfe_name, sizeof(vfe_name), "vfe%d", vfe->id); > | ^ > > Since vfe->id is a u8 I would just increase both the vfe_name and vfe_lite_name > sizes by 1. > > Regards, > > Hans > Hmm. True. Maximum value of VFE id is 255 @ u8 --- bod
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c index db8f68819ded9..f3cf387e4907e 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe.c +++ b/drivers/media/platform/qcom/camss/camss-vfe.c @@ -431,6 +431,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[6]; /* vfeXX\0 */ + char vfe_lite_name[11]; /* vfe_liteXX\0 */ + + 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 @@ -454,9 +468,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; @@ -537,9 +549,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;
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. At the moment we declare vfe_lite0 and vfe_lite1 for sm8250 but never set those clocks because we don't match the strings. 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(-)