| Message ID | 20230911131411.196033-10-bryan.odonoghue@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: qcom: camss: Add parameter passing to remove several outstanding bugs | expand |
Hi Bryan, Thank you for the patch. On Mon, Sep 11, 2023 at 02:14:03PM +0100, Bryan O'Donoghue wrote: > From sdm845 onwards we need to ensure the VFE is powered on prior to > switching on the CSID. > > Currently the code tests for sdm845, sm8250 and then does get/set. This is > not extensible and it turns out is not necessary either since vfe_get and > vfe_set reference count. > > Remove the over-conservative SoC version check. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # rb3 # db410c > --- > drivers/media/platform/qcom/camss/camss-csid.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c > index 99f651e2021cb..02ae3f5cb0c0e 100644 > --- a/drivers/media/platform/qcom/camss/camss-csid.c > +++ b/drivers/media/platform/qcom/camss/camss-csid.c > @@ -159,15 +159,12 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) > struct camss *camss = csid->camss; > struct device *dev = camss->dev; > struct vfe_device *vfe = &camss->vfe[csid->id]; > - u32 version = camss->res->version; > int ret = 0; > > if (on) { > - if (version == CAMSS_8250 || version == CAMSS_845) { > - ret = vfe_get(vfe); > - if (ret < 0) > - return ret; > - } Maybe a comment to explain why we call vfe_get() could be useful ? /* * From SDM845 onwards, the VFE needs to be powered on before * switching on the CSID. Do so unconditionally, as there is no * drawback in following the same powering order on older SoCs. */ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + ret = vfe_get(vfe); > + if (ret < 0) > + return ret; > > ret = pm_runtime_resume_and_get(dev); > if (ret < 0) > @@ -217,8 +214,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) > regulator_bulk_disable(csid->num_supplies, > csid->supplies); > pm_runtime_put_sync(dev); > - if (version == CAMSS_8250 || version == CAMSS_845) > - vfe_put(vfe); > + vfe_put(vfe); > } > > return ret;
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c index 99f651e2021cb..02ae3f5cb0c0e 100644 --- a/drivers/media/platform/qcom/camss/camss-csid.c +++ b/drivers/media/platform/qcom/camss/camss-csid.c @@ -159,15 +159,12 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) struct camss *camss = csid->camss; struct device *dev = camss->dev; struct vfe_device *vfe = &camss->vfe[csid->id]; - u32 version = camss->res->version; int ret = 0; if (on) { - if (version == CAMSS_8250 || version == CAMSS_845) { - ret = vfe_get(vfe); - if (ret < 0) - return ret; - } + ret = vfe_get(vfe); + if (ret < 0) + return ret; ret = pm_runtime_resume_and_get(dev); if (ret < 0) @@ -217,8 +214,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) regulator_bulk_disable(csid->num_supplies, csid->supplies); pm_runtime_put_sync(dev); - if (version == CAMSS_8250 || version == CAMSS_845) - vfe_put(vfe); + vfe_put(vfe); } return ret;