| Message ID | 20241111173845.1773553-1-quic_vikramsa@quicinc.com |
|---|---|
| Headers | show |
| Series | media: qcom: camss: Re-structure camss_link_entities | expand |
On 11/11/2024 17:38, Vikram Sharma wrote: > Refactor the camss_link_entities function by breaking it down into > three distinct functions. Each function will handle the linking of > a specific entity separately, enhancing readability. > > Signed-off-by: Suresh Vankadara <quic_svankada@quicinc.com> > Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > drivers/media/platform/qcom/camss/camss.c | 159 ++++++++++++++-------- > 1 file changed, 105 insertions(+), 54 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index fabe034081ed..1052c01b45f3 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -1840,14 +1840,66 @@ static int camss_init_subdevices(struct camss *camss) > } > > /* > - * camss_link_entities - Register subdev nodes and create links > + * camss_link_entities_csid - Register subdev nodes and create links > * @camss: CAMSS device > * > * Return 0 on success or a negative error code on failure > */ > -static int camss_link_entities(struct camss *camss) > +static int camss_link_entities_csid(struct camss *camss) > { > - int i, j, k; > + int i, j; > + int ret, line_num; > + u16 src_pad; > + u16 sink_pad; > + struct media_entity *src_entity; > + struct media_entity *sink_entity; Vikram. Thanks for the patch. Please reverse Christmas tree this declaration struct media_entity *sink_entity; struct media_entity *src_entity; int ret, line_num; u16 sink_pad; u16 src_pad; int i, j; > + > + for (i = 0; i < camss->res->csid_num; i++) { > + if (camss->ispif) > + line_num = camss->ispif->line_num; > + else > + line_num = camss->vfe[i].res->line_num; > + > + src_entity = &camss->csid[i].subdev.entity; > + for (j = 0; j < line_num; j++) { > + if (camss->ispif) { > + sink_entity = &camss->ispif->line[j].subdev.entity; > + src_pad = MSM_CSID_PAD_SRC; > + sink_pad = MSM_ISPIF_PAD_SINK; > + } else { > + sink_entity = &camss->vfe[i].line[j].subdev.entity; > + src_pad = MSM_CSID_PAD_FIRST_SRC + j; > + sink_pad = MSM_VFE_PAD_SINK; > + } > + > + ret = media_create_pad_link(src_entity, > + src_pad, > + sink_entity, > + sink_pad, > + 0); > + if (ret < 0) { > + dev_err(camss->dev, > + "Failed to link %s->%s entities: %d\n", > + src_entity->name, > + sink_entity->name, > + ret); > + return ret; We repeat this pattern over and over again. I realise that's how it has evolved in this code but since we are going in with the knife we may as well fix this too. Please functionally decompose the "Failed to link" message down into a function. Once both of those are done: Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- bod
Refactor the camss_link_entities function by breaking it down into three distinct functions. Each function will handle the linking of a specific entity separately, enhancing readability. To: Robert Foss <rfoss@kernel.org> To: Todor Tomov <todor.too@gmail.com> To: Bryan O'Donoghue <bryan.odonoghue@linaro.org> To: Krzysztof Kozlowski <krzk+dt@kernel.org> Cc: linux-arm-msm@vger.kernel.org Cc: linux-media@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- Test-by: Vikram Sharma <quic_vikramsa@quicinc.com> Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> Vikram Sharma (1): media: qcom: camss: Restructure camss_link_entities drivers/media/platform/qcom/camss/camss.c | 159 ++++++++++++++-------- 1 file changed, 105 insertions(+), 54 deletions(-) --- Best regards, Vikram Sharma <quic_vikramsa@quicinc.com>