Re: [PATCH v3 07/15] media: qcom: camss: Capture VFE CSID dependency in a helper function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 26/08/2023 11:02, Konrad Dybcio wrote:
On 23.08.2023 12:44, Bryan O'Donoghue wrote:
 From sdm845 onwards we need to ensure the VFE is powered on prior to
switching on the CSID.
And what's the symptom if we fail to ensure this? How can someone
adding support for another platform tell whether the match-list
should be expanded?

That person has to understand the dependency.

The first version of this patch >= SDM845 would mitigate needing to know to expand the list.

Rather than revisit that discussion, I will amend the commit log.



Alternatively we could model up the GDSCs and clocks the CSID needs
without the VFE but, there's a real question of the legitimacy of such a
use-case.

For now drawing a line at sdm845 and switching on the associated VFEs is
a perfectly valid thing to do.

Rather than continually extend out this clause for at least two new SoCs
with this same model - making the vfe_get/vfe_put path start to look
like spaghetti we can encoded the dependency in a helper function.

Use csid_depends_vfe() for this purpose.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
---
  .../media/platform/qcom/camss/camss-csid.c    | 20 +++++++++++++++++--
  1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 08991b070bd61..fd04ed112b564 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -146,6 +146,22 @@ static int csid_set_clock_rates(struct csid_device *csid)
  	return 0;
  }
+static bool csid_depends_vfe(u32 version)
toggle_vfe_before_csid?

If that's clearer np.

+{
+	bool ret = false;
+
+	switch (version) {
+	case CAMSS_845:
+	case CAMSS_8250:
+		ret = true;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
I'm not sure if it would be okay with like C conventions and
stuff, but this can be made shorter by returning from within
the switch statement

Yes but you still need the explicit return at the end of the function or from memory at least some of the compiler/static analysis or checkpatch stuff - I forget which - will complain.

---
bod




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux