[PATCH 0/7] camss: Fixup multiple VDDA regulator support

[PATCH 0/7] camss: Fixup multiple VDDA regulator support

[Bryan O'Donoghue]

Following on from comments from Vladimir on the sm8250 CAMSS patches I had
a look through the RB3 and RB5 schematics.

Here we see the CSI PHY connects to the same VDDA power-rails as USB, PCIe,
UFS and the DSI PHYs.

Right now CAMSS works on SDM845 and SM8250 because one of the USB, PCIe,
or UFS has enabled the relevant VDDA supplies, prior to the CAMSS driver
running.

The solution is to
- Fix the describing YAML
- Add in regulator_bulk_enable()/regulator_bulk_disable()
- Update the DTS to point at the necessary regulators

I have an SDM660 board on-order so when it arrives I can also look into the
vdda_sec regulator if nobody else has.

For now this series addresses the fundamental gap in the CSI PHY power
rails and remediates the situation for the two boards I have schematics for
and can test, RB3/SDM845 and RB5/SM8250.

This patch applies on top of
git.linuxtv.org/hverkuil/media_tree.git  / br-v5.17j

Bootable and testable tree for both RB3 and RB5 here:
git.linaro.org/people/bryan.odonoghue/kernel.git / br-v5.17j+camss-fixes

Bryan O'Donoghue (7):
  media: dt-bindings: media: camss: Fixup vdda regulator descriptions
    sdm845
  media: dt-bindings: media: camss: Add vdda supply declarations sm8250
  arm64: dts: qcom: sdm845: Rename camss vdda-supply to vdda-phy-supply
  arm64: dts: qcom: sdm845: Add camss vdda-pll-supply
  media: camss: Add regulator_bulk support
  media: camss: Point sdm845 at the correct vdda regulators
  media: camss: Point sm8250 at the correct vdda regulators

 .../bindings/media/qcom,sdm845-camss.yaml     | 14 ++-
 .../bindings/media/qcom,sm8250-camss.yaml     | 13 +++
 arch/arm64/boot/dts/qcom/sdm845-db845c.dts    |  3 +-
 .../media/platform/qcom/camss/camss-csid.c    | 40 +++++---
 .../media/platform/qcom/camss/camss-csid.h    |  3 +-
 drivers/media/platform/qcom/camss/camss.c     | 94 +++++++++----------
 drivers/media/platform/qcom/camss/camss.h     |  2 +-
 7 files changed, 100 insertions(+), 69 deletions(-)

-- 
2.33.0

[PATCH 1/7] media: dt-bindings: media: camss: Fixup vdda regulator descriptions sdm845

[Bryan O'Donoghue]

If we review the schematic for RB3 Thundercomm document Turbox-845 we see
that the CAMSS CSI PHY has the same basic power-rail layout as UFS, PCIe
and USB PHYs.

We should therefore have two regulator declarations as is the case for UFS,
PCIe and USB.

Cc: devicetree@vger.kernel.org
Cc: robh@kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../bindings/media/qcom,sdm845-camss.yaml          | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml
index 9ca5dfa7f2260..ae0642b9ae5ec 100644
--- a/Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sdm845-camss.yaml
@@ -203,9 +203,13 @@ properties:
       - const: vfe1
       - const: vfe_lite
 
-  vdda-supply:
+  vdda-phy-supply:
     description:
-      Definition of the regulator used as analog power supply.
+      Phandle to a regulator supply to PHY core block.
+
+  vdda-pll-supply:
+    description:
+      Phandle to 1.8V regulator supply to PHY refclk pll block.
 
 required:
   - clock-names
@@ -217,7 +221,8 @@ required:
   - power-domains
   - reg
   - reg-names
-  - vdda-supply
+  - vdda-phy-supply
+  - vdda-pll-supply
 
 additionalProperties: false
 
@@ -361,7 +366,8 @@ examples:
           "vfe1",
           "vfe_lite";
 
-        vdda-supply = <&reg_2v8>;
+        vdda-phy-supply = <&vreg_l1a_0p875>;
+        vdda-pll-supply = <&vreg_l26a_1p2>;
 
         ports {
           #address-cells = <1>;
-- 
2.33.0

[PATCH 2/7] media: dt-bindings: media: camss: Add vdda supply declarations sm8250

[Bryan O'Donoghue]

Add in missing vdda-phy-supply and vdda-pll-supply declarations. The
sm8250 USB, PCIe, UFS, DSI and CSI PHYs use a common set of vdda rails.
Define the CSI vdda regulators in the same way the qmp PHY does.

Cc: devicetree@vger.kernel.org
Cc: robh@kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../bindings/media/qcom,sm8250-camss.yaml           | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
index af877d61b607d..07a2af12f37df 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml
@@ -265,6 +265,14 @@ properties:
       - const: vfe_lite0
       - const: vfe_lite1
 
+  vdda-phy-supply:
+    description:
+      Phandle to a regulator supply to PHY core block.
+
+  vdda-pll-supply:
+    description:
+      Phandle to 1.8V regulator supply to PHY refclk pll block.
+
 required:
   - clock-names
   - clocks
@@ -277,6 +285,8 @@ required:
   - power-domains
   - reg
   - reg-names
+  - vdda-phy-supply
+  - vdda-pll-supply
 
 additionalProperties: false
 
@@ -316,6 +326,9 @@ examples:
                         "vfe_lite0",
                         "vfe_lite1";
 
+            vdda-phy-supply = <&vreg_l5a_0p88>;
+            vdda-pll-supply = <&vreg_l9a_1p2>;
+
             interrupts = <GIC_SPI 477 IRQ_TYPE_LEVEL_HIGH>,
                          <GIC_SPI 478 IRQ_TYPE_LEVEL_HIGH>,
                          <GIC_SPI 479 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.33.0

[PATCH 3/7] arm64: dts: qcom: sdm845: Rename camss vdda-supply to vdda-phy-supply

[Bryan O'Donoghue]

The dts entry vdda-supply connects to a common vdda-phy-supply rail. Rename
to reflect what the functionality is.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
index 13f80a0b6faaa..c4db88dbf8766 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
@@ -1115,7 +1115,7 @@ &cci {
 };
 
 &camss {
-	vdda-supply = <&vreg_l1a_0p875>;
+	vdda-phy-supply = <&vreg_l1a_0p875>;
 
 	status = "ok";
 
-- 
2.33.0

[PATCH 4/7] arm64: dts: qcom: sdm845: Add camss vdda-pll-supply

[Bryan O'Donoghue]

Add in the missing vdda-pll-supply rail description.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
index c4db88dbf8766..f7bfd69b13620 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
@@ -1116,6 +1116,7 @@ &cci {
 
 &camss {
 	vdda-phy-supply = <&vreg_l1a_0p875>;
+	vdda-pll-supply = <&vreg_l26a_1p2>;
 
 	status = "ok";
 
-- 
2.33.0

[PATCH 5/7] media: camss: Add regulator_bulk support

[Bryan O'Donoghue]

Add the ability to enable or disable multiple regulators in bulk with
camss. This is useful for sm8250, sdm845 and it looks like sdm660 where we
have more than one CSI regulator to do at once.

It should just work for standalone existing vdda regulators and parts which
don't have an explicitly defined CSI regulator.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reported-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 .../media/platform/qcom/camss/camss-csid.c    | 40 +++++---
 .../media/platform/qcom/camss/camss-csid.h    |  3 +-
 drivers/media/platform/qcom/camss/camss.c     | 94 +++++++++----------
 drivers/media/platform/qcom/camss/camss.h     |  2 +-
 4 files changed, 75 insertions(+), 64 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 32f82e471bae1..786a18bd9571c 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -173,7 +173,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
 		if (ret < 0)
 			return ret;
 
-		ret = csid->vdda ? regulator_enable(csid->vdda) : 0;
+		ret = regulator_bulk_enable(csid->num_supplies,
+					    csid->supplies);
 		if (ret < 0) {
 			pm_runtime_put_sync(dev);
 			return ret;
@@ -181,16 +182,16 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
 
 		ret = csid_set_clock_rates(csid);
 		if (ret < 0) {
-			if (csid->vdda)
-				regulator_disable(csid->vdda);
+			regulator_bulk_disable(csid->num_supplies,
+					       csid->supplies);
 			pm_runtime_put_sync(dev);
 			return ret;
 		}
 
 		ret = camss_enable_clocks(csid->nclocks, csid->clock, dev);
 		if (ret < 0) {
-			if (csid->vdda)
-				regulator_disable(csid->vdda);
+			regulator_bulk_disable(csid->num_supplies,
+					       csid->supplies);
 			pm_runtime_put_sync(dev);
 			return ret;
 		}
@@ -201,8 +202,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
 		if (ret < 0) {
 			disable_irq(csid->irq);
 			camss_disable_clocks(csid->nclocks, csid->clock);
-			if (csid->vdda)
-				regulator_disable(csid->vdda);
+			regulator_bulk_disable(csid->num_supplies,
+					       csid->supplies);
 			pm_runtime_put_sync(dev);
 			return ret;
 		}
@@ -211,7 +212,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
 	} else {
 		disable_irq(csid->irq);
 		camss_disable_clocks(csid->nclocks, csid->clock);
-		ret = csid->vdda ? regulator_disable(csid->vdda) : 0;
+		regulator_bulk_disable(csid->num_supplies,
+				       csid->supplies);
 		pm_runtime_put_sync(dev);
 		if (version == CAMSS_8250 || version == CAMSS_845)
 			vfe_put(vfe);
@@ -660,14 +662,22 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,
 	}
 
 	/* Regulator */
-
-	csid->vdda = NULL;
-	if (res->regulator[0])
-		csid->vdda = devm_regulator_get(dev, res->regulator[0]);
-	if (IS_ERR(csid->vdda)) {
-		dev_err(dev, "could not get regulator\n");
-		return PTR_ERR(csid->vdda);
+	for (i = 0; i < ARRAY_SIZE(res->regulators); i++) {
+		if (res->regulators[i])
+			csid->num_supplies++;
 	}
+	csid->supplies = devm_kmalloc_array(camss->dev, csid->num_supplies,
+					    sizeof(csid->supplies), GFP_KERNEL);
+	if (!csid->supplies)
+		return -ENOMEM;
+
+	for (i = 0; i < csid->num_supplies; i++)
+		csid->supplies[i].supply = res->regulators[i];
+
+	ret = devm_regulator_bulk_get(camss->dev, csid->num_supplies,
+				      csid->supplies);
+	if (ret)
+		return ret;
 
 	init_completion(&csid->reset_complete);
 
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index 17a50fa426be1..f06040e44c515 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -152,7 +152,8 @@ struct csid_device {
 	char irq_name[30];
 	struct camss_clock *clock;
 	int nclocks;
-	struct regulator *vdda;
+	struct regulator_bulk_data *supplies;
+	int num_supplies;
 	struct completion reset_complete;
 	struct csid_testgen_config testgen;
 	struct csid_phy_config phy;
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index d9905e737d88d..419c48c4f1d52 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -34,7 +34,7 @@
 static const struct resources csiphy_res_8x16[] = {
 	/* CSIPHY0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer" },
 		.clock_rate = { { 0 },
 				{ 0 },
@@ -46,7 +46,7 @@ static const struct resources csiphy_res_8x16[] = {
 
 	/* CSIPHY1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy1_timer" },
 		.clock_rate = { { 0 },
 				{ 0 },
@@ -60,7 +60,7 @@ static const struct resources csiphy_res_8x16[] = {
 static const struct resources csid_res_8x16[] = {
 	/* CSID0 */
 	{
-		.regulator = { "vdda" },
+		.regulators = { "vdda" },
 		.clock = { "top_ahb", "ispif_ahb", "csi0_ahb", "ahb",
 			   "csi0", "csi0_phy", "csi0_pix", "csi0_rdi" },
 		.clock_rate = { { 0 },
@@ -77,7 +77,7 @@ static const struct resources csid_res_8x16[] = {
 
 	/* CSID1 */
 	{
-		.regulator = { "vdda" },
+		.regulators = { "vdda" },
 		.clock = { "top_ahb", "ispif_ahb", "csi1_ahb", "ahb",
 			   "csi1", "csi1_phy", "csi1_pix", "csi1_rdi" },
 		.clock_rate = { { 0 },
@@ -107,7 +107,7 @@ static const struct resources_ispif ispif_res_8x16 = {
 static const struct resources vfe_res_8x16[] = {
 	/* VFE0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "vfe0", "csi_vfe0",
 			   "vfe_ahb", "vfe_axi", "ahb" },
 		.clock_rate = { { 0 },
@@ -129,7 +129,7 @@ static const struct resources vfe_res_8x16[] = {
 static const struct resources csiphy_res_8x96[] = {
 	/* CSIPHY0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer" },
 		.clock_rate = { { 0 },
 				{ 0 },
@@ -141,7 +141,7 @@ static const struct resources csiphy_res_8x96[] = {
 
 	/* CSIPHY1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy1_timer" },
 		.clock_rate = { { 0 },
 				{ 0 },
@@ -153,7 +153,7 @@ static const struct resources csiphy_res_8x96[] = {
 
 	/* CSIPHY2 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy2_timer" },
 		.clock_rate = { { 0 },
 				{ 0 },
@@ -167,7 +167,7 @@ static const struct resources csiphy_res_8x96[] = {
 static const struct resources csid_res_8x96[] = {
 	/* CSID0 */
 	{
-		.regulator = { "vdda" },
+		.regulators = { "vdda" },
 		.clock = { "top_ahb", "ispif_ahb", "csi0_ahb", "ahb",
 			   "csi0", "csi0_phy", "csi0_pix", "csi0_rdi" },
 		.clock_rate = { { 0 },
@@ -184,7 +184,7 @@ static const struct resources csid_res_8x96[] = {
 
 	/* CSID1 */
 	{
-		.regulator = { "vdda" },
+		.regulators = { "vdda" },
 		.clock = { "top_ahb", "ispif_ahb", "csi1_ahb", "ahb",
 			   "csi1", "csi1_phy", "csi1_pix", "csi1_rdi" },
 		.clock_rate = { { 0 },
@@ -201,7 +201,7 @@ static const struct resources csid_res_8x96[] = {
 
 	/* CSID2 */
 	{
-		.regulator = { "vdda" },
+		.regulators = { "vdda" },
 		.clock = { "top_ahb", "ispif_ahb", "csi2_ahb", "ahb",
 			   "csi2", "csi2_phy", "csi2_pix", "csi2_rdi" },
 		.clock_rate = { { 0 },
@@ -218,7 +218,7 @@ static const struct resources csid_res_8x96[] = {
 
 	/* CSID3 */
 	{
-		.regulator = { "vdda" },
+		.regulators = { "vdda" },
 		.clock = { "top_ahb", "ispif_ahb", "csi3_ahb", "ahb",
 			   "csi3", "csi3_phy", "csi3_pix", "csi3_rdi" },
 		.clock_rate = { { 0 },
@@ -249,7 +249,7 @@ static const struct resources_ispif ispif_res_8x96 = {
 static const struct resources vfe_res_8x96[] = {
 	/* VFE0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ahb", "vfe0", "csi_vfe0", "vfe_ahb",
 			   "vfe0_ahb", "vfe_axi", "vfe0_stream"},
 		.clock_rate = { { 0 },
@@ -267,7 +267,7 @@ static const struct resources vfe_res_8x96[] = {
 
 	/* VFE1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ahb", "vfe1", "csi_vfe1", "vfe_ahb",
 			   "vfe1_ahb", "vfe_axi", "vfe1_stream"},
 		.clock_rate = { { 0 },
@@ -287,7 +287,7 @@ static const struct resources vfe_res_8x96[] = {
 static const struct resources csiphy_res_660[] = {
 	/* CSIPHY0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer",
 			   "csi0_phy", "csiphy_ahb2crif" },
 		.clock_rate = { { 0 },
@@ -301,7 +301,7 @@ static const struct resources csiphy_res_660[] = {
 
 	/* CSIPHY1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy1_timer",
 			   "csi1_phy", "csiphy_ahb2crif" },
 		.clock_rate = { { 0 },
@@ -315,7 +315,7 @@ static const struct resources csiphy_res_660[] = {
 
 	/* CSIPHY2 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy2_timer",
 			   "csi2_phy", "csiphy_ahb2crif" },
 		.clock_rate = { { 0 },
@@ -331,7 +331,7 @@ static const struct resources csiphy_res_660[] = {
 static const struct resources csid_res_660[] = {
 	/* CSID0 */
 	{
-		.regulator = { "vdda", "vdd_sec" },
+		.regulators = { "vdda", "vdd_sec" },
 		.clock = { "top_ahb", "ispif_ahb", "csi0_ahb", "ahb",
 			   "csi0", "csi0_phy", "csi0_pix", "csi0_rdi",
 			   "cphy_csid0" },
@@ -351,7 +351,7 @@ static const struct resources csid_res_660[] = {
 
 	/* CSID1 */
 	{
-		.regulator = { "vdda", "vdd_sec" },
+		.regulators = { "vdda", "vdd_sec" },
 		.clock = { "top_ahb", "ispif_ahb", "csi1_ahb", "ahb",
 			   "csi1", "csi1_phy", "csi1_pix", "csi1_rdi",
 			   "cphy_csid1" },
@@ -371,7 +371,7 @@ static const struct resources csid_res_660[] = {
 
 	/* CSID2 */
 	{
-		.regulator = { "vdda", "vdd_sec" },
+		.regulators = { "vdda", "vdd_sec" },
 		.clock = { "top_ahb", "ispif_ahb", "csi2_ahb", "ahb",
 			   "csi2", "csi2_phy", "csi2_pix", "csi2_rdi",
 			   "cphy_csid2" },
@@ -391,7 +391,7 @@ static const struct resources csid_res_660[] = {
 
 	/* CSID3 */
 	{
-		.regulator = { "vdda", "vdd_sec" },
+		.regulators = { "vdda", "vdd_sec" },
 		.clock = { "top_ahb", "ispif_ahb", "csi3_ahb", "ahb",
 			   "csi3", "csi3_phy", "csi3_pix", "csi3_rdi",
 			   "cphy_csid3" },
@@ -425,7 +425,7 @@ static const struct resources_ispif ispif_res_660 = {
 static const struct resources vfe_res_660[] = {
 	/* VFE0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "throttle_axi", "top_ahb", "ahb", "vfe0",
 			   "csi_vfe0", "vfe_ahb", "vfe0_ahb", "vfe_axi",
 			   "vfe0_stream"},
@@ -446,7 +446,7 @@ static const struct resources vfe_res_660[] = {
 
 	/* VFE1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "throttle_axi", "top_ahb", "ahb", "vfe1",
 			   "csi_vfe1", "vfe_ahb", "vfe1_ahb", "vfe_axi",
 			   "vfe1_stream"},
@@ -469,7 +469,7 @@ static const struct resources vfe_res_660[] = {
 static const struct resources csiphy_res_845[] = {
 	/* CSIPHY0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi", "soc_ahb", "slow_ahb_src",
 				"cpas_ahb", "cphy_rx_src", "csiphy0",
 				"csiphy0_timer_src", "csiphy0_timer" },
@@ -487,7 +487,7 @@ static const struct resources csiphy_res_845[] = {
 
 	/* CSIPHY1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi", "soc_ahb", "slow_ahb_src",
 				"cpas_ahb", "cphy_rx_src", "csiphy1",
 				"csiphy1_timer_src", "csiphy1_timer" },
@@ -505,7 +505,7 @@ static const struct resources csiphy_res_845[] = {
 
 	/* CSIPHY2 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi", "soc_ahb", "slow_ahb_src",
 				"cpas_ahb", "cphy_rx_src", "csiphy2",
 				"csiphy2_timer_src", "csiphy2_timer" },
@@ -523,7 +523,7 @@ static const struct resources csiphy_res_845[] = {
 
 	/* CSIPHY3 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi", "soc_ahb", "slow_ahb_src",
 				"cpas_ahb", "cphy_rx_src", "csiphy3",
 				"csiphy3_timer_src", "csiphy3_timer" },
@@ -543,7 +543,7 @@ static const struct resources csiphy_res_845[] = {
 static const struct resources csid_res_845[] = {
 	/* CSID0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src",
 				"soc_ahb", "vfe0", "vfe0_src",
 				"vfe0_cphy_rx", "csi0",
@@ -563,7 +563,7 @@ static const struct resources csid_res_845[] = {
 
 	/* CSID1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src",
 				"soc_ahb", "vfe1", "vfe1_src",
 				"vfe1_cphy_rx", "csi1",
@@ -583,7 +583,7 @@ static const struct resources csid_res_845[] = {
 
 	/* CSID2 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src",
 				"soc_ahb", "vfe_lite", "vfe_lite_src",
 				"vfe_lite_cphy_rx", "csi2",
@@ -605,7 +605,7 @@ static const struct resources csid_res_845[] = {
 static const struct resources vfe_res_845[] = {
 	/* VFE0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi", "cpas_ahb", "slow_ahb_src",
 				"soc_ahb", "vfe0", "vfe0_axi",
 				"vfe0_src", "csi0",
@@ -625,7 +625,7 @@ static const struct resources vfe_res_845[] = {
 
 	/* VFE1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi", "cpas_ahb", "slow_ahb_src",
 				"soc_ahb", "vfe1", "vfe1_axi",
 				"vfe1_src", "csi1",
@@ -645,7 +645,7 @@ static const struct resources vfe_res_845[] = {
 
 	/* VFE-lite */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi", "cpas_ahb", "slow_ahb_src",
 				"soc_ahb", "vfe_lite",
 				"vfe_lite_src", "csi2",
@@ -666,7 +666,7 @@ static const struct resources vfe_res_845[] = {
 static const struct resources csiphy_res_8250[] = {
 	/* CSIPHY0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "csiphy0", "csiphy0_timer" },
 		.clock_rate = { { 400000000 },
 				{ 300000000 } },
@@ -675,7 +675,7 @@ static const struct resources csiphy_res_8250[] = {
 	},
 	/* CSIPHY1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "csiphy1", "csiphy1_timer" },
 		.clock_rate = { { 400000000 },
 				{ 300000000 } },
@@ -684,7 +684,7 @@ static const struct resources csiphy_res_8250[] = {
 	},
 	/* CSIPHY2 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "csiphy2", "csiphy2_timer" },
 		.clock_rate = { { 400000000 },
 				{ 300000000 } },
@@ -693,7 +693,7 @@ static const struct resources csiphy_res_8250[] = {
 	},
 	/* CSIPHY3 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "csiphy3", "csiphy3_timer" },
 		.clock_rate = { { 400000000 },
 				{ 300000000 } },
@@ -702,7 +702,7 @@ static const struct resources csiphy_res_8250[] = {
 	},
 	/* CSIPHY4 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "csiphy4", "csiphy4_timer" },
 		.clock_rate = { { 400000000 },
 				{ 300000000 } },
@@ -711,7 +711,7 @@ static const struct resources csiphy_res_8250[] = {
 	},
 	/* CSIPHY5 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "csiphy5", "csiphy5_timer" },
 		.clock_rate = { { 400000000 },
 				{ 300000000 } },
@@ -723,7 +723,7 @@ static const struct resources csiphy_res_8250[] = {
 static const struct resources csid_res_8250[] = {
 	/* CSID0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "vfe0_csid", "vfe0_cphy_rx", "vfe0", "vfe0_areg", "vfe0_ahb" },
 		.clock_rate = { { 400000000 },
 				{ 400000000 },
@@ -735,7 +735,7 @@ static const struct resources csid_res_8250[] = {
 	},
 	/* CSID1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "vfe1_csid", "vfe1_cphy_rx", "vfe1", "vfe1_areg", "vfe1_ahb" },
 		.clock_rate = { { 400000000 },
 				{ 400000000 },
@@ -747,7 +747,7 @@ static const struct resources csid_res_8250[] = {
 	},
 	/* CSID2 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx", "vfe_lite",  "vfe_lite_ahb" },
 		.clock_rate = { { 400000000 },
 				{ 400000000 },
@@ -758,7 +758,7 @@ static const struct resources csid_res_8250[] = {
 	},
 	/* CSID3 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx", "vfe_lite",  "vfe_lite_ahb" },
 		.clock_rate = { { 400000000 },
 				{ 400000000 },
@@ -772,7 +772,7 @@ static const struct resources csid_res_8250[] = {
 static const struct resources vfe_res_8250[] = {
 	/* VFE0 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
 			   "camnoc_axi", "vfe0_ahb", "vfe0_areg", "vfe0",
 			   "vfe0_axi", "cam_hf_axi" },
@@ -790,7 +790,7 @@ static const struct resources vfe_res_8250[] = {
 	},
 	/* VFE1 */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
 			   "camnoc_axi", "vfe1_ahb", "vfe1_areg", "vfe1",
 			   "vfe1_axi", "cam_hf_axi" },
@@ -808,7 +808,7 @@ static const struct resources vfe_res_8250[] = {
 	},
 	/* VFE2 (lite) */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
 			   "camnoc_axi", "vfe_lite_ahb", "vfe_lite_axi",
 			   "vfe_lite", "cam_hf_axi" },
@@ -825,7 +825,7 @@ static const struct resources vfe_res_8250[] = {
 	},
 	/* VFE3 (lite) */
 	{
-		.regulator = { NULL },
+		.regulators = { NULL },
 		.clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
 			   "camnoc_axi", "vfe_lite_ahb", "vfe_lite_axi",
 			   "vfe_lite", "cam_hf_axi" },
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index 9c644e638a948..c9b3e0df5be8f 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -42,7 +42,7 @@
 #define CAMSS_RES_MAX 17
 
 struct resources {
-	char *regulator[CAMSS_RES_MAX];
+	char *regulators[CAMSS_RES_MAX];
 	char *clock[CAMSS_RES_MAX];
 	u32 clock_rate[CAMSS_RES_MAX][CAMSS_RES_MAX];
 	char *reg[CAMSS_RES_MAX];
-- 
2.33.0

Re: [PATCH 5/7] media: camss: Add regulator_bulk support

[Bjorn Andersson]

On Thu 06 Jan 15:55 PST 2022, Bryan O'Donoghue wrote:

> Add the ability to enable or disable multiple regulators in bulk with
> camss. This is useful for sm8250, sdm845 and it looks like sdm660 where we
> have more than one CSI regulator to do at once.
> 
> It should just work for standalone existing vdda regulators and parts which
> don't have an explicitly defined CSI regulator.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Reported-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  .../media/platform/qcom/camss/camss-csid.c    | 40 +++++---
>  .../media/platform/qcom/camss/camss-csid.h    |  3 +-
>  drivers/media/platform/qcom/camss/camss.c     | 94 +++++++++----------
>  drivers/media/platform/qcom/camss/camss.h     |  2 +-
>  4 files changed, 75 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index 32f82e471bae1..786a18bd9571c 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
> @@ -173,7 +173,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
>  		if (ret < 0)
>  			return ret;
>  
> -		ret = csid->vdda ? regulator_enable(csid->vdda) : 0;
> +		ret = regulator_bulk_enable(csid->num_supplies,
> +					    csid->supplies);
>  		if (ret < 0) {
>  			pm_runtime_put_sync(dev);
>  			return ret;
> @@ -181,16 +182,16 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
>  
>  		ret = csid_set_clock_rates(csid);
>  		if (ret < 0) {
> -			if (csid->vdda)
> -				regulator_disable(csid->vdda);
> +			regulator_bulk_disable(csid->num_supplies,
> +					       csid->supplies);
>  			pm_runtime_put_sync(dev);
>  			return ret;
>  		}
>  
>  		ret = camss_enable_clocks(csid->nclocks, csid->clock, dev);
>  		if (ret < 0) {
> -			if (csid->vdda)
> -				regulator_disable(csid->vdda);
> +			regulator_bulk_disable(csid->num_supplies,
> +					       csid->supplies);
>  			pm_runtime_put_sync(dev);
>  			return ret;
>  		}
> @@ -201,8 +202,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
>  		if (ret < 0) {
>  			disable_irq(csid->irq);
>  			camss_disable_clocks(csid->nclocks, csid->clock);
> -			if (csid->vdda)
> -				regulator_disable(csid->vdda);
> +			regulator_bulk_disable(csid->num_supplies,
> +					       csid->supplies);
>  			pm_runtime_put_sync(dev);
>  			return ret;
>  		}
> @@ -211,7 +212,8 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
>  	} else {
>  		disable_irq(csid->irq);
>  		camss_disable_clocks(csid->nclocks, csid->clock);
> -		ret = csid->vdda ? regulator_disable(csid->vdda) : 0;
> +		regulator_bulk_disable(csid->num_supplies,
> +				       csid->supplies);
>  		pm_runtime_put_sync(dev);
>  		if (version == CAMSS_8250 || version == CAMSS_845)
>  			vfe_put(vfe);
> @@ -660,14 +662,22 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,
>  	}
>  
>  	/* Regulator */
> -
> -	csid->vdda = NULL;
> -	if (res->regulator[0])
> -		csid->vdda = devm_regulator_get(dev, res->regulator[0]);
> -	if (IS_ERR(csid->vdda)) {
> -		dev_err(dev, "could not get regulator\n");
> -		return PTR_ERR(csid->vdda);
> +	for (i = 0; i < ARRAY_SIZE(res->regulators); i++) {
> +		if (res->regulators[i])
> +			csid->num_supplies++;
>  	}
> +	csid->supplies = devm_kmalloc_array(camss->dev, csid->num_supplies,
> +					    sizeof(csid->supplies), GFP_KERNEL);

When res->regulators is empty num_supplies will be 0 and you will get
ZERO_SIZE_PTR back here, so this works just fine - but I had to read the
code to learn what kzalloc(0) returns.

Perhaps it would be nice to be explicit and make the allocation
conditional on num_supplies? And leave csid->supplies NULL when this
happens?

> +	if (!csid->supplies)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < csid->num_supplies; i++)
> +		csid->supplies[i].supply = res->regulators[i];
> +
> +	ret = devm_regulator_bulk_get(camss->dev, csid->num_supplies,
> +				      csid->supplies);
> +	if (ret)
> +		return ret;
>  
>  	init_completion(&csid->reset_complete);
>  
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
> index 17a50fa426be1..f06040e44c515 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.h
> +++ b/drivers/media/platform/qcom/camss/camss-csid.h
> @@ -152,7 +152,8 @@ struct csid_device {
>  	char irq_name[30];
>  	struct camss_clock *clock;
>  	int nclocks;
> -	struct regulator *vdda;
> +	struct regulator_bulk_data *supplies;
> +	int num_supplies;
>  	struct completion reset_complete;
>  	struct csid_testgen_config testgen;
>  	struct csid_phy_config phy;
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index d9905e737d88d..419c48c4f1d52 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -34,7 +34,7 @@
>  static const struct resources csiphy_res_8x16[] = {
>  	/* CSIPHY0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },

To me this indicates that regulators is an array of 1 element and we set
it to NULL, which based on how above is written must have been the case
at some point.

But I would have preferred this was written as:

		.regulators = {},

Perhaps the maintainer can chime in with a verdict if this would be okay
to do as part of this change?

Regards,
Bjorn

>  		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer" },
>  		.clock_rate = { { 0 },
>  				{ 0 },
> @@ -46,7 +46,7 @@ static const struct resources csiphy_res_8x16[] = {
>  
>  	/* CSIPHY1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy1_timer" },
>  		.clock_rate = { { 0 },
>  				{ 0 },
> @@ -60,7 +60,7 @@ static const struct resources csiphy_res_8x16[] = {
>  static const struct resources csid_res_8x16[] = {
>  	/* CSID0 */
>  	{
> -		.regulator = { "vdda" },
> +		.regulators = { "vdda" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi0_ahb", "ahb",
>  			   "csi0", "csi0_phy", "csi0_pix", "csi0_rdi" },
>  		.clock_rate = { { 0 },
> @@ -77,7 +77,7 @@ static const struct resources csid_res_8x16[] = {
>  
>  	/* CSID1 */
>  	{
> -		.regulator = { "vdda" },
> +		.regulators = { "vdda" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi1_ahb", "ahb",
>  			   "csi1", "csi1_phy", "csi1_pix", "csi1_rdi" },
>  		.clock_rate = { { 0 },
> @@ -107,7 +107,7 @@ static const struct resources_ispif ispif_res_8x16 = {
>  static const struct resources vfe_res_8x16[] = {
>  	/* VFE0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "vfe0", "csi_vfe0",
>  			   "vfe_ahb", "vfe_axi", "ahb" },
>  		.clock_rate = { { 0 },
> @@ -129,7 +129,7 @@ static const struct resources vfe_res_8x16[] = {
>  static const struct resources csiphy_res_8x96[] = {
>  	/* CSIPHY0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer" },
>  		.clock_rate = { { 0 },
>  				{ 0 },
> @@ -141,7 +141,7 @@ static const struct resources csiphy_res_8x96[] = {
>  
>  	/* CSIPHY1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy1_timer" },
>  		.clock_rate = { { 0 },
>  				{ 0 },
> @@ -153,7 +153,7 @@ static const struct resources csiphy_res_8x96[] = {
>  
>  	/* CSIPHY2 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy2_timer" },
>  		.clock_rate = { { 0 },
>  				{ 0 },
> @@ -167,7 +167,7 @@ static const struct resources csiphy_res_8x96[] = {
>  static const struct resources csid_res_8x96[] = {
>  	/* CSID0 */
>  	{
> -		.regulator = { "vdda" },
> +		.regulators = { "vdda" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi0_ahb", "ahb",
>  			   "csi0", "csi0_phy", "csi0_pix", "csi0_rdi" },
>  		.clock_rate = { { 0 },
> @@ -184,7 +184,7 @@ static const struct resources csid_res_8x96[] = {
>  
>  	/* CSID1 */
>  	{
> -		.regulator = { "vdda" },
> +		.regulators = { "vdda" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi1_ahb", "ahb",
>  			   "csi1", "csi1_phy", "csi1_pix", "csi1_rdi" },
>  		.clock_rate = { { 0 },
> @@ -201,7 +201,7 @@ static const struct resources csid_res_8x96[] = {
>  
>  	/* CSID2 */
>  	{
> -		.regulator = { "vdda" },
> +		.regulators = { "vdda" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi2_ahb", "ahb",
>  			   "csi2", "csi2_phy", "csi2_pix", "csi2_rdi" },
>  		.clock_rate = { { 0 },
> @@ -218,7 +218,7 @@ static const struct resources csid_res_8x96[] = {
>  
>  	/* CSID3 */
>  	{
> -		.regulator = { "vdda" },
> +		.regulators = { "vdda" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi3_ahb", "ahb",
>  			   "csi3", "csi3_phy", "csi3_pix", "csi3_rdi" },
>  		.clock_rate = { { 0 },
> @@ -249,7 +249,7 @@ static const struct resources_ispif ispif_res_8x96 = {
>  static const struct resources vfe_res_8x96[] = {
>  	/* VFE0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "ahb", "vfe0", "csi_vfe0", "vfe_ahb",
>  			   "vfe0_ahb", "vfe_axi", "vfe0_stream"},
>  		.clock_rate = { { 0 },
> @@ -267,7 +267,7 @@ static const struct resources vfe_res_8x96[] = {
>  
>  	/* VFE1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "ahb", "vfe1", "csi_vfe1", "vfe_ahb",
>  			   "vfe1_ahb", "vfe_axi", "vfe1_stream"},
>  		.clock_rate = { { 0 },
> @@ -287,7 +287,7 @@ static const struct resources vfe_res_8x96[] = {
>  static const struct resources csiphy_res_660[] = {
>  	/* CSIPHY0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy0_timer",
>  			   "csi0_phy", "csiphy_ahb2crif" },
>  		.clock_rate = { { 0 },
> @@ -301,7 +301,7 @@ static const struct resources csiphy_res_660[] = {
>  
>  	/* CSIPHY1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy1_timer",
>  			   "csi1_phy", "csiphy_ahb2crif" },
>  		.clock_rate = { { 0 },
> @@ -315,7 +315,7 @@ static const struct resources csiphy_res_660[] = {
>  
>  	/* CSIPHY2 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "top_ahb", "ispif_ahb", "ahb", "csiphy2_timer",
>  			   "csi2_phy", "csiphy_ahb2crif" },
>  		.clock_rate = { { 0 },
> @@ -331,7 +331,7 @@ static const struct resources csiphy_res_660[] = {
>  static const struct resources csid_res_660[] = {
>  	/* CSID0 */
>  	{
> -		.regulator = { "vdda", "vdd_sec" },
> +		.regulators = { "vdda", "vdd_sec" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi0_ahb", "ahb",
>  			   "csi0", "csi0_phy", "csi0_pix", "csi0_rdi",
>  			   "cphy_csid0" },
> @@ -351,7 +351,7 @@ static const struct resources csid_res_660[] = {
>  
>  	/* CSID1 */
>  	{
> -		.regulator = { "vdda", "vdd_sec" },
> +		.regulators = { "vdda", "vdd_sec" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi1_ahb", "ahb",
>  			   "csi1", "csi1_phy", "csi1_pix", "csi1_rdi",
>  			   "cphy_csid1" },
> @@ -371,7 +371,7 @@ static const struct resources csid_res_660[] = {
>  
>  	/* CSID2 */
>  	{
> -		.regulator = { "vdda", "vdd_sec" },
> +		.regulators = { "vdda", "vdd_sec" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi2_ahb", "ahb",
>  			   "csi2", "csi2_phy", "csi2_pix", "csi2_rdi",
>  			   "cphy_csid2" },
> @@ -391,7 +391,7 @@ static const struct resources csid_res_660[] = {
>  
>  	/* CSID3 */
>  	{
> -		.regulator = { "vdda", "vdd_sec" },
> +		.regulators = { "vdda", "vdd_sec" },
>  		.clock = { "top_ahb", "ispif_ahb", "csi3_ahb", "ahb",
>  			   "csi3", "csi3_phy", "csi3_pix", "csi3_rdi",
>  			   "cphy_csid3" },
> @@ -425,7 +425,7 @@ static const struct resources_ispif ispif_res_660 = {
>  static const struct resources vfe_res_660[] = {
>  	/* VFE0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "throttle_axi", "top_ahb", "ahb", "vfe0",
>  			   "csi_vfe0", "vfe_ahb", "vfe0_ahb", "vfe_axi",
>  			   "vfe0_stream"},
> @@ -446,7 +446,7 @@ static const struct resources vfe_res_660[] = {
>  
>  	/* VFE1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "throttle_axi", "top_ahb", "ahb", "vfe1",
>  			   "csi_vfe1", "vfe_ahb", "vfe1_ahb", "vfe_axi",
>  			   "vfe1_stream"},
> @@ -469,7 +469,7 @@ static const struct resources vfe_res_660[] = {
>  static const struct resources csiphy_res_845[] = {
>  	/* CSIPHY0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi", "soc_ahb", "slow_ahb_src",
>  				"cpas_ahb", "cphy_rx_src", "csiphy0",
>  				"csiphy0_timer_src", "csiphy0_timer" },
> @@ -487,7 +487,7 @@ static const struct resources csiphy_res_845[] = {
>  
>  	/* CSIPHY1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi", "soc_ahb", "slow_ahb_src",
>  				"cpas_ahb", "cphy_rx_src", "csiphy1",
>  				"csiphy1_timer_src", "csiphy1_timer" },
> @@ -505,7 +505,7 @@ static const struct resources csiphy_res_845[] = {
>  
>  	/* CSIPHY2 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi", "soc_ahb", "slow_ahb_src",
>  				"cpas_ahb", "cphy_rx_src", "csiphy2",
>  				"csiphy2_timer_src", "csiphy2_timer" },
> @@ -523,7 +523,7 @@ static const struct resources csiphy_res_845[] = {
>  
>  	/* CSIPHY3 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi", "soc_ahb", "slow_ahb_src",
>  				"cpas_ahb", "cphy_rx_src", "csiphy3",
>  				"csiphy3_timer_src", "csiphy3_timer" },
> @@ -543,7 +543,7 @@ static const struct resources csiphy_res_845[] = {
>  static const struct resources csid_res_845[] = {
>  	/* CSID0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src",
>  				"soc_ahb", "vfe0", "vfe0_src",
>  				"vfe0_cphy_rx", "csi0",
> @@ -563,7 +563,7 @@ static const struct resources csid_res_845[] = {
>  
>  	/* CSID1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src",
>  				"soc_ahb", "vfe1", "vfe1_src",
>  				"vfe1_cphy_rx", "csi1",
> @@ -583,7 +583,7 @@ static const struct resources csid_res_845[] = {
>  
>  	/* CSID2 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src",
>  				"soc_ahb", "vfe_lite", "vfe_lite_src",
>  				"vfe_lite_cphy_rx", "csi2",
> @@ -605,7 +605,7 @@ static const struct resources csid_res_845[] = {
>  static const struct resources vfe_res_845[] = {
>  	/* VFE0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi", "cpas_ahb", "slow_ahb_src",
>  				"soc_ahb", "vfe0", "vfe0_axi",
>  				"vfe0_src", "csi0",
> @@ -625,7 +625,7 @@ static const struct resources vfe_res_845[] = {
>  
>  	/* VFE1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi", "cpas_ahb", "slow_ahb_src",
>  				"soc_ahb", "vfe1", "vfe1_axi",
>  				"vfe1_src", "csi1",
> @@ -645,7 +645,7 @@ static const struct resources vfe_res_845[] = {
>  
>  	/* VFE-lite */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi", "cpas_ahb", "slow_ahb_src",
>  				"soc_ahb", "vfe_lite",
>  				"vfe_lite_src", "csi2",
> @@ -666,7 +666,7 @@ static const struct resources vfe_res_845[] = {
>  static const struct resources csiphy_res_8250[] = {
>  	/* CSIPHY0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "csiphy0", "csiphy0_timer" },
>  		.clock_rate = { { 400000000 },
>  				{ 300000000 } },
> @@ -675,7 +675,7 @@ static const struct resources csiphy_res_8250[] = {
>  	},
>  	/* CSIPHY1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "csiphy1", "csiphy1_timer" },
>  		.clock_rate = { { 400000000 },
>  				{ 300000000 } },
> @@ -684,7 +684,7 @@ static const struct resources csiphy_res_8250[] = {
>  	},
>  	/* CSIPHY2 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "csiphy2", "csiphy2_timer" },
>  		.clock_rate = { { 400000000 },
>  				{ 300000000 } },
> @@ -693,7 +693,7 @@ static const struct resources csiphy_res_8250[] = {
>  	},
>  	/* CSIPHY3 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "csiphy3", "csiphy3_timer" },
>  		.clock_rate = { { 400000000 },
>  				{ 300000000 } },
> @@ -702,7 +702,7 @@ static const struct resources csiphy_res_8250[] = {
>  	},
>  	/* CSIPHY4 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "csiphy4", "csiphy4_timer" },
>  		.clock_rate = { { 400000000 },
>  				{ 300000000 } },
> @@ -711,7 +711,7 @@ static const struct resources csiphy_res_8250[] = {
>  	},
>  	/* CSIPHY5 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "csiphy5", "csiphy5_timer" },
>  		.clock_rate = { { 400000000 },
>  				{ 300000000 } },
> @@ -723,7 +723,7 @@ static const struct resources csiphy_res_8250[] = {
>  static const struct resources csid_res_8250[] = {
>  	/* CSID0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "vfe0_csid", "vfe0_cphy_rx", "vfe0", "vfe0_areg", "vfe0_ahb" },
>  		.clock_rate = { { 400000000 },
>  				{ 400000000 },
> @@ -735,7 +735,7 @@ static const struct resources csid_res_8250[] = {
>  	},
>  	/* CSID1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "vfe1_csid", "vfe1_cphy_rx", "vfe1", "vfe1_areg", "vfe1_ahb" },
>  		.clock_rate = { { 400000000 },
>  				{ 400000000 },
> @@ -747,7 +747,7 @@ static const struct resources csid_res_8250[] = {
>  	},
>  	/* CSID2 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx", "vfe_lite",  "vfe_lite_ahb" },
>  		.clock_rate = { { 400000000 },
>  				{ 400000000 },
> @@ -758,7 +758,7 @@ static const struct resources csid_res_8250[] = {
>  	},
>  	/* CSID3 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx", "vfe_lite",  "vfe_lite_ahb" },
>  		.clock_rate = { { 400000000 },
>  				{ 400000000 },
> @@ -772,7 +772,7 @@ static const struct resources csid_res_8250[] = {
>  static const struct resources vfe_res_8250[] = {
>  	/* VFE0 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
>  			   "camnoc_axi", "vfe0_ahb", "vfe0_areg", "vfe0",
>  			   "vfe0_axi", "cam_hf_axi" },
> @@ -790,7 +790,7 @@ static const struct resources vfe_res_8250[] = {
>  	},
>  	/* VFE1 */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
>  			   "camnoc_axi", "vfe1_ahb", "vfe1_areg", "vfe1",
>  			   "vfe1_axi", "cam_hf_axi" },
> @@ -808,7 +808,7 @@ static const struct resources vfe_res_8250[] = {
>  	},
>  	/* VFE2 (lite) */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
>  			   "camnoc_axi", "vfe_lite_ahb", "vfe_lite_axi",
>  			   "vfe_lite", "cam_hf_axi" },
> @@ -825,7 +825,7 @@ static const struct resources vfe_res_8250[] = {
>  	},
>  	/* VFE3 (lite) */
>  	{
> -		.regulator = { NULL },
> +		.regulators = { NULL },
>  		.clock = { "camnoc_axi_src", "slow_ahb_src", "cpas_ahb",
>  			   "camnoc_axi", "vfe_lite_ahb", "vfe_lite_axi",
>  			   "vfe_lite", "cam_hf_axi" },
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index 9c644e638a948..c9b3e0df5be8f 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -42,7 +42,7 @@
>  #define CAMSS_RES_MAX 17
>  
>  struct resources {
> -	char *regulator[CAMSS_RES_MAX];
> +	char *regulators[CAMSS_RES_MAX];
>  	char *clock[CAMSS_RES_MAX];
>  	u32 clock_rate[CAMSS_RES_MAX][CAMSS_RES_MAX];
>  	char *reg[CAMSS_RES_MAX];
> -- 
> 2.33.0
> 

Re: [PATCH 5/7] media: camss: Add regulator_bulk support

[Bryan O'Donoghue]

On 07/01/2022 19:52, Bjorn Andersson wrote:
> When res->regulators is empty num_supplies will be 0 and you will get
> ZERO_SIZE_PTR back here, so this works just fine - but I had to read the
> code to learn what kzalloc(0) returns.

Literally as soon as I hit send I realised I hadn't tested the NULL 
case, then did test it and found this behavior.

> Perhaps it would be nice to be explicit and make the allocation
> conditional on num_supplies? And leave csid->supplies NULL when this
> happens?

I'll rewrite it, agreed its confusing to look at.

---
bod

[PATCH 6/7] media: camss: Point sdm845 at the correct vdda regulators

[Bryan O'Donoghue]

Reviewing the RB3 schematic its clear that we have missed out on defining
one of the power-rails associated with the CSI PHY.

Other PHYs such as the UFS, PCIe and USB connect to these rails and define
each regulator individually.

This means if we were to switch off the other various PHYs which enable
these rails, the CAMSS would not appropriately power-on the CSI PHY.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 419c48c4f1d52..6de257502e88b 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -543,7 +543,7 @@ static const struct resources csiphy_res_845[] = {
 static const struct resources csid_res_845[] = {
 	/* CSID0 */
 	{
-		.regulators = { NULL },
+		.regulators = { "vdda-phy", "vdda-pll" },
 		.clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src",
 				"soc_ahb", "vfe0", "vfe0_src",
 				"vfe0_cphy_rx", "csi0",
@@ -563,7 +563,7 @@ static const struct resources csid_res_845[] = {
 
 	/* CSID1 */
 	{
-		.regulators = { NULL },
+		.regulators = { "vdda-phy", "vdda-pll" },
 		.clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src",
 				"soc_ahb", "vfe1", "vfe1_src",
 				"vfe1_cphy_rx", "csi1",
@@ -583,7 +583,7 @@ static const struct resources csid_res_845[] = {
 
 	/* CSID2 */
 	{
-		.regulators = { NULL },
+		.regulators = { "vdda-phy", "vdda-pll" },
 		.clock = { "cpas_ahb", "cphy_rx_src", "slow_ahb_src",
 				"soc_ahb", "vfe_lite", "vfe_lite_src",
 				"vfe_lite_cphy_rx", "csi2",
-- 
2.33.0

[PATCH 7/7] media: camss: Point sm8250 at the correct vdda regulators

[Bryan O'Donoghue]

Reviewing the RB5 schematic its clear that we have missed out on defining
both of the power-rails associated with the CSI PHY.

Other PHYs such as the UFS, PCIe and USB connect to these rails and define
each regulator individually.

This means if we were to switch off the other various PHYs which enable
these rails, the CAMSS would not appropriately power-on the CSI PHY.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 6de257502e88b..412c8ee7c182d 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -723,7 +723,7 @@ static const struct resources csiphy_res_8250[] = {
 static const struct resources csid_res_8250[] = {
 	/* CSID0 */
 	{
-		.regulators = { NULL },
+		.regulators = { "vdda-phy", "vdda-pll" },
 		.clock = { "vfe0_csid", "vfe0_cphy_rx", "vfe0", "vfe0_areg", "vfe0_ahb" },
 		.clock_rate = { { 400000000 },
 				{ 400000000 },
@@ -735,7 +735,7 @@ static const struct resources csid_res_8250[] = {
 	},
 	/* CSID1 */
 	{
-		.regulators = { NULL },
+		.regulators = { "vdda-phy", "vdda-pll" },
 		.clock = { "vfe1_csid", "vfe1_cphy_rx", "vfe1", "vfe1_areg", "vfe1_ahb" },
 		.clock_rate = { { 400000000 },
 				{ 400000000 },
@@ -747,7 +747,7 @@ static const struct resources csid_res_8250[] = {
 	},
 	/* CSID2 */
 	{
-		.regulators = { NULL },
+		.regulators = { "vdda-phy", "vdda-pll" },
 		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx", "vfe_lite",  "vfe_lite_ahb" },
 		.clock_rate = { { 400000000 },
 				{ 400000000 },
@@ -758,7 +758,7 @@ static const struct resources csid_res_8250[] = {
 	},
 	/* CSID3 */
 	{
-		.regulators = { NULL },
+		.regulators = { "vdda-phy", "vdda-pll" },
 		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx", "vfe_lite",  "vfe_lite_ahb" },
 		.clock_rate = { { 400000000 },
 				{ 400000000 },
-- 
2.33.0