diff mbox series

[1/2] clk: qcom: camcc-sm8250: Fix absent mmcx regulator reference

Message ID 20210708010839.692242-2-bryan.odonoghue@linaro.org
State New
Headers show
Series Fix and enable camcc-sm8250 | expand

Commit Message

Bryan O'Donoghue July 8, 2021, 1:08 a.m. UTC
The current implementation omits the necessary mmcx supply, which means if
you are running the code for this block and a prior clock driver, like say
the videocc hasn't run, then a reset will be generated the first time we
touch these registers.

Fixes: 5d66ca79b58c ("clk: qcom: Add camera clock controller driver for SM8250")
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

---
 drivers/clk/qcom/camcc-sm8250.c | 6 ++++++
 1 file changed, 6 insertions(+)

-- 
2.30.1

Comments

Bjorn Andersson July 8, 2021, 4:03 a.m. UTC | #1
On Wed 07 Jul 20:08 CDT 2021, Bryan O'Donoghue wrote:

> The current implementation omits the necessary mmcx supply, which means if

> you are running the code for this block and a prior clock driver, like say

> the videocc hasn't run, then a reset will be generated the first time we

> touch these registers.

> 

> Fixes: 5d66ca79b58c ("clk: qcom: Add camera clock controller driver for SM8250")

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>


camcc isn't enabled in the upstream dts yet and I expect that we're
going to conclude on defining these GDSCs as subdomains of the cc's
power-domain in time for v5.15.

I don't mind if Stephen picks this to make sure the driver is
functional, but I will hold off on the dts change.

Regards,
Bjorn

> ---

>  drivers/clk/qcom/camcc-sm8250.c | 6 ++++++

>  1 file changed, 6 insertions(+)

> 

> diff --git a/drivers/clk/qcom/camcc-sm8250.c b/drivers/clk/qcom/camcc-sm8250.c

> index 439eaafdcc86..c51112546bfc 100644

> --- a/drivers/clk/qcom/camcc-sm8250.c

> +++ b/drivers/clk/qcom/camcc-sm8250.c

> @@ -2212,6 +2212,7 @@ static struct gdsc bps_gdsc = {

>  	},

>  	.flags = HW_CTRL | POLL_CFG_GDSCR,

>  	.pwrsts = PWRSTS_OFF_ON,

> +	.supply = "mmcx",

>  };

>  

>  static struct gdsc ipe_0_gdsc = {

> @@ -2221,6 +2222,7 @@ static struct gdsc ipe_0_gdsc = {

>  	},

>  	.flags = HW_CTRL | POLL_CFG_GDSCR,

>  	.pwrsts = PWRSTS_OFF_ON,

> +	.supply = "mmcx",

>  };

>  

>  static struct gdsc sbi_gdsc = {

> @@ -2230,6 +2232,7 @@ static struct gdsc sbi_gdsc = {

>  	},

>  	.flags = HW_CTRL | POLL_CFG_GDSCR,

>  	.pwrsts = PWRSTS_OFF_ON,

> +	.supply = "mmcx",

>  };

>  

>  static struct gdsc ife_0_gdsc = {

> @@ -2239,6 +2242,7 @@ static struct gdsc ife_0_gdsc = {

>  	},

>  	.flags = POLL_CFG_GDSCR,

>  	.pwrsts = PWRSTS_OFF_ON,

> +	.supply = "mmcx",

>  };

>  

>  static struct gdsc ife_1_gdsc = {

> @@ -2248,6 +2252,7 @@ static struct gdsc ife_1_gdsc = {

>  	},

>  	.flags = POLL_CFG_GDSCR,

>  	.pwrsts = PWRSTS_OFF_ON,

> +	.supply = "mmcx",

>  };

>  

>  static struct gdsc titan_top_gdsc = {

> @@ -2257,6 +2262,7 @@ static struct gdsc titan_top_gdsc = {

>  	},

>  	.flags = POLL_CFG_GDSCR,

>  	.pwrsts = PWRSTS_OFF_ON,

> +	.supply = "mmcx",

>  };

>  

>  static struct clk_regmap *cam_cc_sm8250_clocks[] = {

> -- 

> 2.30.1

>
Stephen Boyd July 27, 2021, 6:29 p.m. UTC | #2
Quoting Bjorn Andersson (2021-07-07 21:03:06)
> On Wed 07 Jul 20:08 CDT 2021, Bryan O'Donoghue wrote:

> 

> > The current implementation omits the necessary mmcx supply, which means if

> > you are running the code for this block and a prior clock driver, like say

> > the videocc hasn't run, then a reset will be generated the first time we

> > touch these registers.

> > 

> > Fixes: 5d66ca79b58c ("clk: qcom: Add camera clock controller driver for SM8250")

> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

> 

> camcc isn't enabled in the upstream dts yet and I expect that we're

> going to conclude on defining these GDSCs as subdomains of the cc's

> power-domain in time for v5.15.

> 

> I don't mind if Stephen picks this to make sure the driver is

> functional, but I will hold off on the dts change.

> 


Seems like this is superseded by the GDSC patches that use subdomains
instead of a fake supply?
Bryan O'Donoghue July 27, 2021, 6:31 p.m. UTC | #3
On 27/07/2021 19:29, Stephen Boyd wrote:
> Quoting Bjorn Andersson (2021-07-07 21:03:06)

>> On Wed 07 Jul 20:08 CDT 2021, Bryan O'Donoghue wrote:

>>

>>> The current implementation omits the necessary mmcx supply, which means if

>>> you are running the code for this block and a prior clock driver, like say

>>> the videocc hasn't run, then a reset will be generated the first time we

>>> touch these registers.

>>>

>>> Fixes: 5d66ca79b58c ("clk: qcom: Add camera clock controller driver for SM8250")

>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

>>

>> camcc isn't enabled in the upstream dts yet and I expect that we're

>> going to conclude on defining these GDSCs as subdomains of the cc's

>> power-domain in time for v5.15.

>>

>> I don't mind if Stephen picks this to make sure the driver is

>> functional, but I will hold off on the dts change.

>>

> 

> Seems like this is superseded by the GDSC patches that use subdomains

> instead of a fake supply?

> 

yep
diff mbox series

Patch

diff --git a/drivers/clk/qcom/camcc-sm8250.c b/drivers/clk/qcom/camcc-sm8250.c
index 439eaafdcc86..c51112546bfc 100644
--- a/drivers/clk/qcom/camcc-sm8250.c
+++ b/drivers/clk/qcom/camcc-sm8250.c
@@ -2212,6 +2212,7 @@  static struct gdsc bps_gdsc = {
 	},
 	.flags = HW_CTRL | POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct gdsc ipe_0_gdsc = {
@@ -2221,6 +2222,7 @@  static struct gdsc ipe_0_gdsc = {
 	},
 	.flags = HW_CTRL | POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct gdsc sbi_gdsc = {
@@ -2230,6 +2232,7 @@  static struct gdsc sbi_gdsc = {
 	},
 	.flags = HW_CTRL | POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct gdsc ife_0_gdsc = {
@@ -2239,6 +2242,7 @@  static struct gdsc ife_0_gdsc = {
 	},
 	.flags = POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct gdsc ife_1_gdsc = {
@@ -2248,6 +2252,7 @@  static struct gdsc ife_1_gdsc = {
 	},
 	.flags = POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct gdsc titan_top_gdsc = {
@@ -2257,6 +2262,7 @@  static struct gdsc titan_top_gdsc = {
 	},
 	.flags = POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
+	.supply = "mmcx",
 };
 
 static struct clk_regmap *cam_cc_sm8250_clocks[] = {