| Message ID | 20241125103457.1970608-1-quic_vikramsa@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: qcom: camss: Re-structure camss_link_entities | expand |
Hi Vikram, On 11/25/24 12:34, 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. > > Changes in V3: > - Broke down the change in 2 patches. first one to functionally > decompose link error message. second to restrcture the link > function. > - Removed the declarion of camss_link_error from header file. > - Link to v2: https://lore.kernel.org/linux-arm-msm/20241112133846.2397017-1-quic_vikramsa@quicinc.com/ as I said last time I don't see the value of these changes. Since the changes are non-functional, then hopefully there should be no issues with them, however I really miss the point of adding 65 lines of code for a questionable reason and at the price of increased complexity. Is there a good reason not to drop the series? > Changes in V2: > - Declared variables in reverse christmas tree order. > - Functionally decomposed link error message. > - Link to v1: https://lore.kernel.org/linux-arm-msm/20241111173845.1773553-1-quic_vikramsa@quicinc.com/ > > To: Robert Foss <rfoss@kernel.org> > To: Todor Tomov <todor.too@gmail.com> > To: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Cc: linux-media@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > > Vikram Sharma (2): > media: qcom: camss: reducing the repitious error message string > media: qcom: camss: Restructure camss_link_entities > > drivers/media/platform/qcom/camss/camss.c | 195 ++++++++++++++-------- > 1 file changed, 130 insertions(+), 65 deletions(-) > -- Best wishes, Vladimir
On 25/11/2024 11:57, Vladimir Zapolskiy wrote: > Hi Vikram, > > On 11/25/24 12:34, 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. >> >> Changes in V3: >> - Broke down the change in 2 patches. first one to functionally >> decompose link error message. second to restrcture the link >> function. >> - Removed the declarion of camss_link_error from header file. >> - Link to v2: https://lore.kernel.org/linux-arm- >> msm/20241112133846.2397017-1-quic_vikramsa@quicinc.com/ > > as I said last time I don't see the value of these changes. > > Since the changes are non-functional, then hopefully there should be > no issues with them, however I really miss the point of adding 65 > lines of code for a questionable reason and at the price of increased > complexity. > > Is there a good reason not to drop the series? I think there is value in both functional decomposition and tidying up code - for example removing circuitous if/elses in favour of more discreet and easy to read functions, so I'm inclined to accept this series. --- 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. Changes in V3: - Broke down the change in 2 patches. first one to functionally decompose link error message. second to restrcture the link function. - Removed the declarion of camss_link_error from header file. - Link to v2: https://lore.kernel.org/linux-arm-msm/20241112133846.2397017-1-quic_vikramsa@quicinc.com/ Changes in V2: - Declared variables in reverse christmas tree order. - Functionally decomposed link error message. - Link to v1: https://lore.kernel.org/linux-arm-msm/20241111173845.1773553-1-quic_vikramsa@quicinc.com/ To: Robert Foss <rfoss@kernel.org> To: Todor Tomov <todor.too@gmail.com> To: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Cc: linux-media@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> Vikram Sharma (2): media: qcom: camss: reducing the repitious error message string media: qcom: camss: Restructure camss_link_entities drivers/media/platform/qcom/camss/camss.c | 195 ++++++++++++++-------- 1 file changed, 130 insertions(+), 65 deletions(-)