dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] drm/msm/dsi regulator improvements
@ 2022-07-26 17:38 Douglas Anderson
  2022-07-26 17:38 ` [PATCH v2 1/7] drm/msm/dsi: Fix number of regulators for msm8996_dsi_cfg Douglas Anderson
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Douglas Anderson @ 2022-07-26 17:38 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Mark Brown
  Cc: Sean Paul, Vinod Koul, Archit Taneja, Loic Poulain,
	Jonathan Marek, Liam Girdwood, David Airlie, linux-arm-msm,
	Vladimir Lypak, Konrad Dybcio, Douglas Anderson, dri-devel,
	Bjorn Andersson, Rajeev Nandan, Marijn Suijten,
	AngeloGioacchino Del Regno, José Expósito,
	Stephen Boyd, freedreno, linux-kernel

The main goal of this series is to make a small dent in cleaning up
the way we deal with regulator loads. The idea is to add some extra
functionality to the regulator "bulk" API so that consumers can
specify the load using that. Though I didn't convert everyone over, I
include patches in this series that show how the Qualcomm DSI driver
is improved by this.

I'd expect:
* The first two patches are bugfixes found while converting the DSI
  driver over. Those could land any time.
* The third patch ("drm/msm/dsi: Don't set a load before disabling a
  regulator") is a patch a sent the other day verbatim, included in
  this series because it's highly related. It could land any
  time. That's why I called this series "v2".
* After that I have patches that add to the regulator API and then
  show a usage of those in the DSI driver. I'd expect that the two
  regulator patches could land in the regulator tree. The DSI patches
  would need to wait until the new regulator changes are available.

Changes in v2:
- ("Fix number of regulators for msm8996_dsi_cfg") new for v2.
- ("Fix number of regulators for SDM660") new for v2.
- ("Allow specifying an initial load w/ the bulk API") new for v2.
- ("Use the new regulator bulk feature to specify the load") new for v2.
- ("Allow drivers to define their init data as const") new for v2.
- ("Take advantage of devm_regulator_bulk_get_const") new for v2.

Douglas Anderson (7):
  drm/msm/dsi: Fix number of regulators for msm8996_dsi_cfg
  drm/msm/dsi: Fix number of regulators for SDM660
  drm/msm/dsi: Don't set a load before disabling a regulator
  regulator: core: Allow specifying an initial load w/ the bulk API
  drm/msm/dsi: Use the new regulator bulk feature to specify the load
  regulator: core: Allow drivers to define their init data as const
  drm/msm/dsi: Take advantage of devm_regulator_bulk_get_const()

 drivers/gpu/drm/msm/dsi/dsi.h                 |   1 -
 drivers/gpu/drm/msm/dsi/dsi_cfg.c             | 172 +++++++++---------
 drivers/gpu/drm/msm/dsi/dsi_cfg.h             |   3 +-
 drivers/gpu/drm/msm/dsi/dsi_host.c            |  61 +------
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c         |  41 +----
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c    |   4 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c    |   6 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c    |   4 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c    |   6 +-
 .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c   |   2 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c     |   6 +-
 drivers/regulator/core.c                      |  20 +-
 drivers/regulator/devres.c                    |  28 +++
 include/linux/regulator/consumer.h            |  16 +-
 14 files changed, 165 insertions(+), 205 deletions(-)

-- 
2.37.1.359.gd136c6c3e2-goog


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/7] drm/msm/dsi: Fix number of regulators for msm8996_dsi_cfg
  2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
@ 2022-07-26 17:38 ` Douglas Anderson
  2022-07-26 17:38 ` [PATCH v2 2/7] drm/msm/dsi: Fix number of regulators for SDM660 Douglas Anderson
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Douglas Anderson @ 2022-07-26 17:38 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Mark Brown
  Cc: Sean Paul, Archit Taneja, Loic Poulain, David Airlie,
	linux-arm-msm, Konrad Dybcio, Douglas Anderson, dri-devel,
	linux-kernel, Rajeev Nandan, freedreno

3 regulators are specified listed but the number 2 is specified. Fix
it.

Fixes: 3a3ff88a0fc1 ("drm/msm/dsi: Add 8x96 info in dsi_cfg")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- ("Fix number of regulators for msm8996_dsi_cfg") new for v2.

 drivers/gpu/drm/msm/dsi/dsi_cfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 2c23324a2296..02000a7b7a18 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -109,7 +109,7 @@ static const char * const dsi_8996_bus_clk_names[] = {
 static const struct msm_dsi_config msm8996_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
 	.reg_cfg = {
-		.num = 2,
+		.num = 3,
 		.regs = {
 			{"vdda", 18160, 1 },	/* 1.25 V */
 			{"vcca", 17000, 32 },	/* 0.925 V */
-- 
2.37.1.359.gd136c6c3e2-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/7] drm/msm/dsi: Fix number of regulators for SDM660
  2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
  2022-07-26 17:38 ` [PATCH v2 1/7] drm/msm/dsi: Fix number of regulators for msm8996_dsi_cfg Douglas Anderson
@ 2022-07-26 17:38 ` Douglas Anderson
  2022-07-26 17:38 ` [PATCH v2 3/7] drm/msm/dsi: Don't set a load before disabling a regulator Douglas Anderson
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Douglas Anderson @ 2022-07-26 17:38 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Mark Brown
  Cc: Sean Paul, Loic Poulain, David Airlie, linux-arm-msm,
	Konrad Dybcio, Douglas Anderson, dri-devel, linux-kernel,
	Rajeev Nandan, freedreno

1 regulators is specified listed but the number 2 is specified. This
presumably means we try to get a regulator with no name. Fix it.

Fixes: 033f47f7f121 ("drm/msm/dsi: Add DSI configuration for SDM660")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- ("Fix number of regulators for SDM660") new for v2.

 drivers/gpu/drm/msm/dsi/dsi_cfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 02000a7b7a18..72c018e26f47 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -148,7 +148,7 @@ static const char * const dsi_sdm660_bus_clk_names[] = {
 static const struct msm_dsi_config sdm660_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
 	.reg_cfg = {
-		.num = 2,
+		.num = 1,
 		.regs = {
 			{"vdda", 12560, 4 },	/* 1.2 V */
 		},
-- 
2.37.1.359.gd136c6c3e2-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/7] drm/msm/dsi: Don't set a load before disabling a regulator
  2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
  2022-07-26 17:38 ` [PATCH v2 1/7] drm/msm/dsi: Fix number of regulators for msm8996_dsi_cfg Douglas Anderson
  2022-07-26 17:38 ` [PATCH v2 2/7] drm/msm/dsi: Fix number of regulators for SDM660 Douglas Anderson
@ 2022-07-26 17:38 ` Douglas Anderson
  2022-07-26 17:38 ` [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API Douglas Anderson
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Douglas Anderson @ 2022-07-26 17:38 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Mark Brown
  Cc: Sean Paul, Vinod Koul, Loic Poulain, Jonathan Marek,
	David Airlie, linux-arm-msm, Vladimir Lypak, Konrad Dybcio,
	Douglas Anderson, dri-devel, Bjorn Andersson, Rajeev Nandan,
	Marijn Suijten, AngeloGioacchino Del Regno,
	José Expósito, Stephen Boyd, freedreno, linux-kernel

As of commit 5451781dadf8 ("regulator: core: Only count load for
enabled consumers"), a load isn't counted for a disabled
regulator. That means all the code in the DSI driver to specify and
set loads before disabling a regulator is not actually doing anything
useful. Let's remove it.

It should be noted that all of the loads set that were being specified
were pointless noise anyway. The only use for this number is to pick
between low power and high power modes of regulators. Regulators
appear to do this changeover at loads on the order of 10000 uA. You
would a lot of clients of the same rail for that 100 uA number to
count for anything.

Note that now that we get rid of the setting of the load at disable
time, we can just set the load once when we first get the regulator
and then forget it.

It should also be noted that the regulator functions
regulator_bulk_enable() and regulator_set_load() already print error
messages when they encounter problems so while moving things around we
get rid of some extra error prints.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

(no changes since v1)

 drivers/gpu/drm/msm/dsi/dsi.h                 |  1 -
 drivers/gpu/drm/msm/dsi/dsi_cfg.c             | 52 +++++++++----------
 drivers/gpu/drm/msm/dsi/dsi_host.c            | 45 ++++------------
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c         | 46 ++++------------
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c    |  4 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c    |  6 +--
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c    |  4 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c    |  6 +--
 .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c   |  2 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c     |  6 +--
 10 files changed, 60 insertions(+), 112 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 580a1e6358bf..bb6a5bd05cb1 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -37,7 +37,6 @@ enum msm_dsi_phy_usecase {
 struct dsi_reg_entry {
 	char name[32];
 	int enable_load;
-	int disable_load;
 };
 
 struct dsi_reg_config {
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 72c018e26f47..901d6fd53800 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -14,9 +14,9 @@ static const struct msm_dsi_config apq8064_dsi_cfg = {
 	.reg_cfg = {
 		.num = 3,
 		.regs = {
-			{"vdda", 100000, 100},	/* 1.2 V */
-			{"avdd", 10000, 100},	/* 3.0 V */
-			{"vddio", 100000, 100},	/* 1.8 V */
+			{"vdda", 100000},	/* 1.2 V */
+			{"avdd", 10000},	/* 3.0 V */
+			{"vddio", 100000},	/* 1.8 V */
 		},
 	},
 	.bus_clk_names = dsi_v2_bus_clk_names,
@@ -34,9 +34,9 @@ static const struct msm_dsi_config msm8974_apq8084_dsi_cfg = {
 	.reg_cfg = {
 		.num = 3,
 		.regs = {
-			{"vdd", 150000, 100},	/* 3.0 V */
-			{"vdda", 100000, 100},	/* 1.2 V */
-			{"vddio", 100000, 100},	/* 1.8 V */
+			{"vdd", 150000},	/* 3.0 V */
+			{"vdda", 100000},	/* 1.2 V */
+			{"vddio", 100000},	/* 1.8 V */
 		},
 	},
 	.bus_clk_names = dsi_6g_bus_clk_names,
@@ -54,8 +54,8 @@ static const struct msm_dsi_config msm8916_dsi_cfg = {
 	.reg_cfg = {
 		.num = 2,
 		.regs = {
-			{"vdda", 100000, 100},	/* 1.2 V */
-			{"vddio", 100000, 100},	/* 1.8 V */
+			{"vdda", 100000},	/* 1.2 V */
+			{"vddio", 100000},	/* 1.8 V */
 		},
 	},
 	.bus_clk_names = dsi_8916_bus_clk_names,
@@ -73,8 +73,8 @@ static const struct msm_dsi_config msm8976_dsi_cfg = {
 	.reg_cfg = {
 		.num = 2,
 		.regs = {
-			{"vdda", 100000, 100},	/* 1.2 V */
-			{"vddio", 100000, 100},	/* 1.8 V */
+			{"vdda", 100000},	/* 1.2 V */
+			{"vddio", 100000},	/* 1.8 V */
 		},
 	},
 	.bus_clk_names = dsi_8976_bus_clk_names,
@@ -88,12 +88,12 @@ static const struct msm_dsi_config msm8994_dsi_cfg = {
 	.reg_cfg = {
 		.num = 6,
 		.regs = {
-			{"vdda", 100000, 100},	/* 1.25 V */
-			{"vddio", 100000, 100},	/* 1.8 V */
-			{"vcca", 10000, 100},	/* 1.0 V */
-			{"vdd", 100000, 100},	/* 1.8 V */
-			{"lab_reg", -1, -1},
-			{"ibb_reg", -1, -1},
+			{"vdda", 100000},	/* 1.25 V */
+			{"vddio", 100000},	/* 1.8 V */
+			{"vcca", 10000},	/* 1.0 V */
+			{"vdd", 100000},	/* 1.8 V */
+			{"lab_reg", -1},
+			{"ibb_reg", -1},
 		},
 	},
 	.bus_clk_names = dsi_6g_bus_clk_names,
@@ -111,9 +111,9 @@ static const struct msm_dsi_config msm8996_dsi_cfg = {
 	.reg_cfg = {
 		.num = 3,
 		.regs = {
-			{"vdda", 18160, 1 },	/* 1.25 V */
-			{"vcca", 17000, 32 },	/* 0.925 V */
-			{"vddio", 100000, 100 },/* 1.8 V */
+			{"vdda", 18160},	/* 1.25 V */
+			{"vcca", 17000},	/* 0.925 V */
+			{"vddio", 100000},/* 1.8 V */
 		},
 	},
 	.bus_clk_names = dsi_8996_bus_clk_names,
@@ -131,8 +131,8 @@ static const struct msm_dsi_config msm8998_dsi_cfg = {
 	.reg_cfg = {
 		.num = 2,
 		.regs = {
-			{"vdd", 367000, 16 },	/* 0.9 V */
-			{"vdda", 62800, 2 },	/* 1.2 V */
+			{"vdd", 367000},	/* 0.9 V */
+			{"vdda", 62800},	/* 1.2 V */
 		},
 	},
 	.bus_clk_names = dsi_msm8998_bus_clk_names,
@@ -150,7 +150,7 @@ static const struct msm_dsi_config sdm660_dsi_cfg = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdda", 12560, 4 },	/* 1.2 V */
+			{"vdda", 12560},	/* 1.2 V */
 		},
 	},
 	.bus_clk_names = dsi_sdm660_bus_clk_names,
@@ -172,7 +172,7 @@ static const struct msm_dsi_config sdm845_dsi_cfg = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdda", 21800, 4 },	/* 1.2 V */
+			{"vdda", 21800},	/* 1.2 V */
 		},
 	},
 	.bus_clk_names = dsi_sdm845_bus_clk_names,
@@ -186,7 +186,7 @@ static const struct msm_dsi_config sc7180_dsi_cfg = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdda", 21800, 4 },	/* 1.2 V */
+			{"vdda", 21800},	/* 1.2 V */
 		},
 	},
 	.bus_clk_names = dsi_sc7180_bus_clk_names,
@@ -204,7 +204,7 @@ static const struct msm_dsi_config sc7280_dsi_cfg = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdda", 8350, 0 },	/* 1.2 V */
+			{"vdda", 8350},	/* 1.2 V */
 		},
 	},
 	.bus_clk_names = dsi_sc7280_bus_clk_names,
@@ -222,7 +222,7 @@ static const struct msm_dsi_config qcm2290_dsi_cfg = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdda", 21800, 4 },	/* 1.2 V */
+			{"vdda", 21800},	/* 1.2 V */
 		},
 	},
 	.bus_clk_names = dsi_qcm2290_bus_clk_names,
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index a95d5df52653..04265ad2fbef 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -259,15 +259,7 @@ static inline struct msm_dsi_host *to_msm_dsi_host(struct mipi_dsi_host *host)
 static void dsi_host_regulator_disable(struct msm_dsi_host *msm_host)
 {
 	struct regulator_bulk_data *s = msm_host->supplies;
-	const struct dsi_reg_entry *regs = msm_host->cfg_hnd->cfg->reg_cfg.regs;
 	int num = msm_host->cfg_hnd->cfg->reg_cfg.num;
-	int i;
-
-	DBG("");
-	for (i = num - 1; i >= 0; i--)
-		if (regs[i].disable_load >= 0)
-			regulator_set_load(s[i].consumer,
-					   regs[i].disable_load);
 
 	regulator_bulk_disable(num, s);
 }
@@ -275,35 +267,9 @@ static void dsi_host_regulator_disable(struct msm_dsi_host *msm_host)
 static int dsi_host_regulator_enable(struct msm_dsi_host *msm_host)
 {
 	struct regulator_bulk_data *s = msm_host->supplies;
-	const struct dsi_reg_entry *regs = msm_host->cfg_hnd->cfg->reg_cfg.regs;
 	int num = msm_host->cfg_hnd->cfg->reg_cfg.num;
-	int ret, i;
-
-	DBG("");
-	for (i = 0; i < num; i++) {
-		if (regs[i].enable_load >= 0) {
-			ret = regulator_set_load(s[i].consumer,
-						 regs[i].enable_load);
-			if (ret < 0) {
-				pr_err("regulator %d set op mode failed, %d\n",
-					i, ret);
-				goto fail;
-			}
-		}
-	}
-
-	ret = regulator_bulk_enable(num, s);
-	if (ret < 0) {
-		pr_err("regulator enable failed, %d\n", ret);
-		goto fail;
-	}
 
-	return 0;
-
-fail:
-	for (i--; i >= 0; i--)
-		regulator_set_load(s[i].consumer, regs[i].disable_load);
-	return ret;
+	return regulator_bulk_enable(num, s);
 }
 
 static int dsi_regulator_init(struct msm_dsi_host *msm_host)
@@ -323,6 +289,15 @@ static int dsi_regulator_init(struct msm_dsi_host *msm_host)
 		return ret;
 	}
 
+	for (i = 0; i < num; i++) {
+		if (regs[i].enable_load >= 0) {
+			ret = regulator_set_load(s[i].consumer,
+						 regs[i].enable_load);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index a39de3bdc7fa..330c0c4e7f9d 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -529,20 +529,22 @@ static int dsi_phy_regulator_init(struct msm_dsi_phy *phy)
 		return ret;
 	}
 
+	for (i = 0; i < num; i++) {
+		if (regs[i].enable_load >= 0) {
+			ret = regulator_set_load(s[i].consumer,
+							regs[i].enable_load);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
 	return 0;
 }
 
 static void dsi_phy_regulator_disable(struct msm_dsi_phy *phy)
 {
 	struct regulator_bulk_data *s = phy->supplies;
-	const struct dsi_reg_entry *regs = phy->cfg->reg_cfg.regs;
 	int num = phy->cfg->reg_cfg.num;
-	int i;
-
-	DBG("");
-	for (i = num - 1; i >= 0; i--)
-		if (regs[i].disable_load >= 0)
-			regulator_set_load(s[i].consumer, regs[i].disable_load);
 
 	regulator_bulk_disable(num, s);
 }
@@ -550,37 +552,9 @@ static void dsi_phy_regulator_disable(struct msm_dsi_phy *phy)
 static int dsi_phy_regulator_enable(struct msm_dsi_phy *phy)
 {
 	struct regulator_bulk_data *s = phy->supplies;
-	const struct dsi_reg_entry *regs = phy->cfg->reg_cfg.regs;
-	struct device *dev = &phy->pdev->dev;
 	int num = phy->cfg->reg_cfg.num;
-	int ret, i;
 
-	DBG("");
-	for (i = 0; i < num; i++) {
-		if (regs[i].enable_load >= 0) {
-			ret = regulator_set_load(s[i].consumer,
-							regs[i].enable_load);
-			if (ret < 0) {
-				DRM_DEV_ERROR(dev,
-					"regulator %d set op mode failed, %d\n",
-					i, ret);
-				goto fail;
-			}
-		}
-	}
-
-	ret = regulator_bulk_enable(num, s);
-	if (ret < 0) {
-		DRM_DEV_ERROR(dev, "regulator enable failed, %d\n", ret);
-		goto fail;
-	}
-
-	return 0;
-
-fail:
-	for (i--; i >= 0; i--)
-		regulator_set_load(s[i].consumer, regs[i].disable_load);
-	return ret;
+	return regulator_bulk_enable(num, s);
 }
 
 static int dsi_phy_enable_resource(struct msm_dsi_phy *phy)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
index 08b015ea1b1e..6a10a1448051 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
@@ -1033,7 +1033,7 @@ const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdds", 36000, 32},
+			{"vdds", 36000},
 		},
 	},
 	.ops = {
@@ -1055,7 +1055,7 @@ const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdds", 36000, 32},
+			{"vdds", 36000},
 		},
 	},
 	.ops = {
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
index 8199c53567f4..0f3d4c56c333 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
@@ -1029,7 +1029,7 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vcca", 17000, 32},
+			{"vcca", 17000},
 		},
 	},
 	.ops = {
@@ -1050,7 +1050,7 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vcca", 73400, 32},
+			{"vcca", 73400},
 		},
 	},
 	.ops = {
@@ -1071,7 +1071,7 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vcca", 17000, 32},
+			{"vcca", 17000},
 		},
 	},
 	.ops = {
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c
index ee7c418a1c29..b7c621d94981 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c
@@ -134,8 +134,8 @@ const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs = {
 	.reg_cfg = {
 		.num = 2,
 		.regs = {
-			{"vddio", 100000, 100},	/* 1.8 V */
-			{"vcca", 10000, 100},	/* 1.0 V */
+			{"vddio", 100000},	/* 1.8 V */
+			{"vcca", 10000},	/* 1.0 V */
 		},
 	},
 	.ops = {
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
index 48eab80b548e..6beba387640d 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
@@ -774,7 +774,7 @@ const struct msm_dsi_phy_cfg dsi_phy_28nm_hpm_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vddio", 100000, 100},
+			{"vddio", 100000},
 		},
 	},
 	.ops = {
@@ -795,7 +795,7 @@ const struct msm_dsi_phy_cfg dsi_phy_28nm_hpm_famb_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vddio", 100000, 100},
+			{"vddio", 100000},
 		},
 	},
 	.ops = {
@@ -816,7 +816,7 @@ const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vddio", 100000, 100},	/* 1.8 V */
+			{"vddio", 100000},	/* 1.8 V */
 		},
 	},
 	.ops = {
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
index fc56cdcc9ad6..2e942b10fffa 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
@@ -653,7 +653,7 @@ const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vddio", 100000, 100},	/* 1.8 V */
+			{"vddio", 100000},	/* 1.8 V */
 		},
 	},
 	.ops = {
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 66ed1919a1db..9c7c49ce1200 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -1041,7 +1041,7 @@ const struct msm_dsi_phy_cfg dsi_phy_7nm_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdds", 36000, 32},
+			{"vdds", 36000},
 		},
 	},
 	.ops = {
@@ -1068,7 +1068,7 @@ const struct msm_dsi_phy_cfg dsi_phy_7nm_8150_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdds", 36000, 32},
+			{"vdds", 36000},
 		},
 	},
 	.ops = {
@@ -1090,7 +1090,7 @@ const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs = {
 	.reg_cfg = {
 		.num = 1,
 		.regs = {
-			{"vdds", 37550, 0},
+			{"vdds", 37550},
 		},
 	},
 	.ops = {
-- 
2.37.1.359.gd136c6c3e2-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API
  2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
                   ` (2 preceding siblings ...)
  2022-07-26 17:38 ` [PATCH v2 3/7] drm/msm/dsi: Don't set a load before disabling a regulator Douglas Anderson
@ 2022-07-26 17:38 ` Douglas Anderson
  2022-08-16 12:58   ` Yongqin Liu
  2022-07-26 17:38 ` [PATCH v2 5/7] drm/msm/dsi: Use the new regulator bulk feature to specify the load Douglas Anderson
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Douglas Anderson @ 2022-07-26 17:38 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Mark Brown
  Cc: Liam Girdwood, linux-arm-msm, linux-kernel, dri-devel,
	Douglas Anderson, freedreno

There are a number of drivers that follow a pattern that looks like
this:
1. Use the regulator bulk API to get a bunch of regulators.
2. Set the load on each of the regulators to use whenever the
   regulators are enabled.

Let's make this easier by just allowing the drivers to pass the load
in.

As part of this change we need to move the error printing in
regulator_bulk_get() around; let's switch to the new dev_err_probe()
to simplify it.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- ("Allow specifying an initial load w/ the bulk API") new for v2.

 drivers/regulator/core.c           | 20 ++++++++++++--------
 include/linux/regulator/consumer.h | 12 ++++++++----
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1e54a833f2cf..17c476fc8adb 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4783,22 +4783,26 @@ int regulator_bulk_get(struct device *dev, int num_consumers,
 		consumers[i].consumer = regulator_get(dev,
 						      consumers[i].supply);
 		if (IS_ERR(consumers[i].consumer)) {
-			ret = PTR_ERR(consumers[i].consumer);
 			consumers[i].consumer = NULL;
+			ret = dev_err_probe(dev, PTR_ERR(consumers[i].consumer),
+					    "Failed to get supply '%s'",
+					    consumers[i].supply);
 			goto err;
 		}
+
+		if (consumers[i].init_load_uA > 0) {
+			ret = regulator_set_load(consumers[i].consumer,
+						 consumers[i].init_load_uA);
+			if (ret) {
+				i++;
+				goto err;
+			}
+		}
 	}
 
 	return 0;
 
 err:
-	if (ret != -EPROBE_DEFER)
-		dev_err(dev, "Failed to get supply '%s': %pe\n",
-			consumers[i].supply, ERR_PTR(ret));
-	else
-		dev_dbg(dev, "Failed to get supply '%s', deferring\n",
-			consumers[i].supply);
-
 	while (--i >= 0)
 		regulator_put(consumers[i].consumer);
 
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index bbf6590a6dec..5779f4466e62 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -171,10 +171,13 @@ struct regulator;
 /**
  * struct regulator_bulk_data - Data used for bulk regulator operations.
  *
- * @supply:   The name of the supply.  Initialised by the user before
- *            using the bulk regulator APIs.
- * @consumer: The regulator consumer for the supply.  This will be managed
- *            by the bulk API.
+ * @supply:       The name of the supply.  Initialised by the user before
+ *                using the bulk regulator APIs.
+ * @init_load_uA: After getting the regulator, regulator_set_load() will be
+ *                called with this load.  Initialised by the user before
+ *                using the bulk regulator APIs.
+ * @consumer:     The regulator consumer for the supply.  This will be managed
+ *                by the bulk API.
  *
  * The regulator APIs provide a series of regulator_bulk_() API calls as
  * a convenience to consumers which require multiple supplies.  This
@@ -182,6 +185,7 @@ struct regulator;
  */
 struct regulator_bulk_data {
 	const char *supply;
+	int init_load_uA;
 	struct regulator *consumer;
 
 	/* private: Internal use */
-- 
2.37.1.359.gd136c6c3e2-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/7] drm/msm/dsi: Use the new regulator bulk feature to specify the load
  2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
                   ` (3 preceding siblings ...)
  2022-07-26 17:38 ` [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API Douglas Anderson
@ 2022-07-26 17:38 ` Douglas Anderson
  2022-07-26 17:38 ` [PATCH v2 6/7] regulator: core: Allow drivers to define their init data as const Douglas Anderson
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Douglas Anderson @ 2022-07-26 17:38 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Mark Brown
  Cc: Sean Paul, Jonathan Marek, David Airlie, linux-arm-msm,
	Vladimir Lypak, Douglas Anderson, dri-devel, Stephen Boyd,
	Vinod Koul, Rajeev Nandan, freedreno, linux-kernel

As of the patch ("regulator: core: Allow specifying an initial load w/
the bulk API") we can now specify the initial load in the bulk data
rather than having to manually call regulator_set_load() on each
regulator. Let's use it.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- ("Use the new regulator bulk feature to specify the load") new for v2.

 drivers/gpu/drm/msm/dsi/dsi_host.c    | 13 +++----------
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 13 +++----------
 2 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 04265ad2fbef..dec7a94cf819 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -279,8 +279,10 @@ static int dsi_regulator_init(struct msm_dsi_host *msm_host)
 	int num = msm_host->cfg_hnd->cfg->reg_cfg.num;
 	int i, ret;
 
-	for (i = 0; i < num; i++)
+	for (i = 0; i < num; i++) {
 		s[i].supply = regs[i].name;
+		s[i].init_load_uA = regs[i].enable_load;
+	}
 
 	ret = devm_regulator_bulk_get(&msm_host->pdev->dev, num, s);
 	if (ret < 0) {
@@ -289,15 +291,6 @@ static int dsi_regulator_init(struct msm_dsi_host *msm_host)
 		return ret;
 	}
 
-	for (i = 0; i < num; i++) {
-		if (regs[i].enable_load >= 0) {
-			ret = regulator_set_load(s[i].consumer,
-						 regs[i].enable_load);
-			if (ret < 0)
-				return ret;
-		}
-	}
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index 330c0c4e7f9d..f42ff57861da 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -515,8 +515,10 @@ static int dsi_phy_regulator_init(struct msm_dsi_phy *phy)
 	int num = phy->cfg->reg_cfg.num;
 	int i, ret;
 
-	for (i = 0; i < num; i++)
+	for (i = 0; i < num; i++) {
 		s[i].supply = regs[i].name;
+		s[i].init_load_uA = regs[i].enable_load;
+	}
 
 	ret = devm_regulator_bulk_get(dev, num, s);
 	if (ret < 0) {
@@ -529,15 +531,6 @@ static int dsi_phy_regulator_init(struct msm_dsi_phy *phy)
 		return ret;
 	}
 
-	for (i = 0; i < num; i++) {
-		if (regs[i].enable_load >= 0) {
-			ret = regulator_set_load(s[i].consumer,
-							regs[i].enable_load);
-			if (ret < 0)
-				return ret;
-		}
-	}
-
 	return 0;
 }
 
-- 
2.37.1.359.gd136c6c3e2-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/7] regulator: core: Allow drivers to define their init data as const
  2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
                   ` (4 preceding siblings ...)
  2022-07-26 17:38 ` [PATCH v2 5/7] drm/msm/dsi: Use the new regulator bulk feature to specify the load Douglas Anderson
@ 2022-07-26 17:38 ` Douglas Anderson
  2022-07-26 17:38 ` [PATCH v2 7/7] drm/msm/dsi: Take advantage of devm_regulator_bulk_get_const() Douglas Anderson
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Douglas Anderson @ 2022-07-26 17:38 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Mark Brown
  Cc: Liam Girdwood, linux-arm-msm, linux-kernel, dri-devel,
	Douglas Anderson, freedreno

Drivers tend to want to define the names of their regulators somewhere
in their source file as "static const". This means, inevitable, that
every driver out there open codes something like this:

static const char * const supply_names[] = {
 "vcc", "vccl",
};

static int get_regulators(struct my_data *data)
{
  int i;

  data->supplies = devm_kzalloc(...)
  if (!data->supplies)
    return -ENOMEM;

  for (i = 0; i < ARRAY_SIZE(supply_names); i++)
    data->supplies[i].supply = supply_names[i];

  return devm_regulator_bulk_get(data->dev,
                                 ARRAY_SIZE(supply_names),
				 data->supplies);
}

Let's make this more convenient by doing providing a helper that does
the copy.

I have chosen to have the "const" input structure here be the exact
same structure as the normal one passed to
devm_regulator_bulk_get(). This is slightly inefficent since the input
data can't possibly have anything useful for "ret" or consumer and
thus we waste 8 bytes per structure. This seems an OK tradeoff for not
introducing an extra structure.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- ("Allow drivers to define their init data as const") new for v2.

 drivers/regulator/devres.c         | 28 ++++++++++++++++++++++++++++
 include/linux/regulator/consumer.h |  4 ++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 9113233f41cd..32823a87fd40 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -166,6 +166,34 @@ int devm_regulator_bulk_get(struct device *dev, int num_consumers,
 }
 EXPORT_SYMBOL_GPL(devm_regulator_bulk_get);
 
+/**
+ * devm_regulator_bulk_get_const - devm_regulator_bulk_get() w/ const data
+ *
+ * @dev:           device to supply
+ * @num_consumers: number of consumers to register
+ * @in_consumers:  const configuration of consumers
+ * @out_consumers: in_consumers is copied here and this is passed to
+ *		   devm_regulator_bulk_get().
+ *
+ * This is a convenience function to allow bulk regulator configuration
+ * to be stored "static const" in files.
+ *
+ * Return: 0 on success, an errno on failure.
+ */
+int devm_regulator_bulk_get_const(struct device *dev, int num_consumers,
+				  const struct regulator_bulk_data *in_consumers,
+				  struct regulator_bulk_data **out_consumers)
+{
+	*out_consumers = devm_kmemdup(dev, in_consumers,
+				      num_consumers * sizeof(*in_consumers),
+				      GFP_KERNEL);
+	if (*out_consumers == NULL)
+		return -ENOMEM;
+
+	return devm_regulator_bulk_get(dev, num_consumers, *out_consumers);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_bulk_get_const);
+
 static void devm_rdev_release(struct device *dev, void *res)
 {
 	regulator_unregister(*(struct regulator_dev **)res);
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 5779f4466e62..bc6cda706d1f 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -244,6 +244,10 @@ int __must_check regulator_bulk_get(struct device *dev, int num_consumers,
 				    struct regulator_bulk_data *consumers);
 int __must_check devm_regulator_bulk_get(struct device *dev, int num_consumers,
 					 struct regulator_bulk_data *consumers);
+int __must_check devm_regulator_bulk_get_const(
+	struct device *dev, int num_consumers,
+	const struct regulator_bulk_data *in_consumers,
+	struct regulator_bulk_data **out_consumers);
 int __must_check regulator_bulk_enable(int num_consumers,
 				       struct regulator_bulk_data *consumers);
 int regulator_bulk_disable(int num_consumers,
-- 
2.37.1.359.gd136c6c3e2-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 7/7] drm/msm/dsi: Take advantage of devm_regulator_bulk_get_const()
  2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
                   ` (5 preceding siblings ...)
  2022-07-26 17:38 ` [PATCH v2 6/7] regulator: core: Allow drivers to define their init data as const Douglas Anderson
@ 2022-07-26 17:38 ` Douglas Anderson
  2022-07-27 23:02 ` (subset) [PATCH v2 0/7] drm/msm/dsi regulator improvements Mark Brown
  2022-07-28  0:20 ` Mark Brown
  8 siblings, 0 replies; 16+ messages in thread
From: Douglas Anderson @ 2022-07-26 17:38 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Mark Brown
  Cc: Sean Paul, Loic Poulain, Jonathan Marek, David Airlie,
	linux-arm-msm, Konrad Dybcio, Douglas Anderson, dri-devel,
	Stephen Boyd, Vinod Koul, Rajeev Nandan, freedreno, linux-kernel

In the patch ("regulator: core: Allow drivers to define their init
data as const") we no longer need to do copying of regulator bulk data
from initdata to something dynamic. Let's take advantage of that.

In addition to saving some code, this also moves us to using
ARRAY_SIZE() to specify how many regulators we have which is less
error prone.

NOTE: Though I haven't done the math, this is likely an overall
savings in terms of "static const" data. We previously always
allocated space for 8 supplies. Each of these supplies took up 36
bytes of data (32 for name, 4 for an int).

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- ("Take advantage of devm_regulator_bulk_get_const") new for v2.

 drivers/gpu/drm/msm/dsi/dsi_cfg.c  | 172 ++++++++++++++---------------
 drivers/gpu/drm/msm/dsi/dsi_cfg.h  |   3 +-
 drivers/gpu/drm/msm/dsi/dsi_host.c |  27 +----
 3 files changed, 94 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 901d6fd53800..7e97c239ed48 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -9,16 +9,16 @@ static const char * const dsi_v2_bus_clk_names[] = {
 	"core_mmss", "iface", "bus",
 };
 
+static const struct regulator_bulk_data apq8064_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 100000 },	/* 1.2 V */
+	{ .supply = "avdd", .init_load_uA = 10000 },	/* 3.0 V */
+	{ .supply = "vddio", .init_load_uA = 100000 },	/* 1.8 V */
+};
+
 static const struct msm_dsi_config apq8064_dsi_cfg = {
 	.io_offset = 0,
-	.reg_cfg = {
-		.num = 3,
-		.regs = {
-			{"vdda", 100000},	/* 1.2 V */
-			{"avdd", 10000},	/* 3.0 V */
-			{"vddio", 100000},	/* 1.8 V */
-		},
-	},
+	.regulator_data = apq8064_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(apq8064_dsi_regulators),
 	.bus_clk_names = dsi_v2_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_v2_bus_clk_names),
 	.io_start = { 0x4700000, 0x5800000 },
@@ -29,16 +29,16 @@ static const char * const dsi_6g_bus_clk_names[] = {
 	"mdp_core", "iface", "bus", "core_mmss",
 };
 
+static const struct regulator_bulk_data msm8974_apq8084_regulators[] = {
+	{ .supply = "vdd", .init_load_uA = 150000 },	/* 3.0 V */
+	{ .supply = "vdda", .init_load_uA = 100000 },	/* 1.2 V */
+	{ .supply = "vddio", .init_load_uA = 100000 },	/* 1.8 V */
+};
+
 static const struct msm_dsi_config msm8974_apq8084_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 3,
-		.regs = {
-			{"vdd", 150000},	/* 3.0 V */
-			{"vdda", 100000},	/* 1.2 V */
-			{"vddio", 100000},	/* 1.8 V */
-		},
-	},
+	.regulator_data = msm8974_apq8084_regulators,
+	.num_regulators = ARRAY_SIZE(msm8974_apq8084_regulators),
 	.bus_clk_names = dsi_6g_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_6g_bus_clk_names),
 	.io_start = { 0xfd922800, 0xfd922b00 },
@@ -49,15 +49,15 @@ static const char * const dsi_8916_bus_clk_names[] = {
 	"mdp_core", "iface", "bus",
 };
 
+static const struct regulator_bulk_data msm8916_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 100000 },	/* 1.2 V */
+	{ .supply = "vddio", .init_load_uA = 100000 },	/* 1.8 V */
+};
+
 static const struct msm_dsi_config msm8916_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 2,
-		.regs = {
-			{"vdda", 100000},	/* 1.2 V */
-			{"vddio", 100000},	/* 1.8 V */
-		},
-	},
+	.regulator_data = msm8916_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(msm8916_dsi_regulators),
 	.bus_clk_names = dsi_8916_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_8916_bus_clk_names),
 	.io_start = { 0x1a98000 },
@@ -68,34 +68,34 @@ static const char * const dsi_8976_bus_clk_names[] = {
 	"mdp_core", "iface", "bus",
 };
 
+static const struct regulator_bulk_data msm8976_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 100000 },	/* 1.2 V */
+	{ .supply = "vddio", .init_load_uA = 100000 },	/* 1.8 V */
+};
+
 static const struct msm_dsi_config msm8976_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 2,
-		.regs = {
-			{"vdda", 100000},	/* 1.2 V */
-			{"vddio", 100000},	/* 1.8 V */
-		},
-	},
+	.regulator_data = msm8976_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(msm8976_dsi_regulators),
 	.bus_clk_names = dsi_8976_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_8976_bus_clk_names),
 	.io_start = { 0x1a94000, 0x1a96000 },
 	.num_dsi = 2,
 };
 
+static const struct regulator_bulk_data msm8994_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 100000 },	/* 1.25 V */
+	{ .supply = "vddio", .init_load_uA = 100000 },	/* 1.8 V */
+	{ .supply = "vcca", .init_load_uA = 10000 },	/* 1.0 V */
+	{ .supply = "vdd", .init_load_uA = 100000 },	/* 1.8 V */
+	{ .supply = "lab_reg", .init_load_uA = -1 },
+	{ .supply = "ibb_reg", .init_load_uA = -1 },
+};
+
 static const struct msm_dsi_config msm8994_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 6,
-		.regs = {
-			{"vdda", 100000},	/* 1.25 V */
-			{"vddio", 100000},	/* 1.8 V */
-			{"vcca", 10000},	/* 1.0 V */
-			{"vdd", 100000},	/* 1.8 V */
-			{"lab_reg", -1},
-			{"ibb_reg", -1},
-		},
-	},
+	.regulator_data = msm8994_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(msm8994_dsi_regulators),
 	.bus_clk_names = dsi_6g_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_6g_bus_clk_names),
 	.io_start = { 0xfd998000, 0xfd9a0000 },
@@ -106,16 +106,16 @@ static const char * const dsi_8996_bus_clk_names[] = {
 	"mdp_core", "iface", "bus", "core_mmss",
 };
 
+static const struct regulator_bulk_data msm8996_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 18160 },	/* 1.25 V */
+	{ .supply = "vcca", .init_load_uA = 17000 },	/* 0.925 V */
+	{ .supply = "vddio", .init_load_uA = 100000 },	/* 1.8 V */
+};
+
 static const struct msm_dsi_config msm8996_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 3,
-		.regs = {
-			{"vdda", 18160},	/* 1.25 V */
-			{"vcca", 17000},	/* 0.925 V */
-			{"vddio", 100000},/* 1.8 V */
-		},
-	},
+	.regulator_data = msm8996_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(msm8996_dsi_regulators),
 	.bus_clk_names = dsi_8996_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_8996_bus_clk_names),
 	.io_start = { 0x994000, 0x996000 },
@@ -126,15 +126,15 @@ static const char * const dsi_msm8998_bus_clk_names[] = {
 	"iface", "bus", "core",
 };
 
+static const struct regulator_bulk_data msm8998_dsi_regulators[] = {
+	{ .supply = "vdd", .init_load_uA = 367000 },	/* 0.9 V */
+	{ .supply = "vdda", .init_load_uA = 62800 },	/* 1.2 V */
+};
+
 static const struct msm_dsi_config msm8998_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 2,
-		.regs = {
-			{"vdd", 367000},	/* 0.9 V */
-			{"vdda", 62800},	/* 1.2 V */
-		},
-	},
+	.regulator_data = msm8998_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(msm8998_dsi_regulators),
 	.bus_clk_names = dsi_msm8998_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_msm8998_bus_clk_names),
 	.io_start = { 0xc994000, 0xc996000 },
@@ -145,14 +145,14 @@ static const char * const dsi_sdm660_bus_clk_names[] = {
 	"iface", "bus", "core", "core_mmss",
 };
 
+static const struct regulator_bulk_data sdm660_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 12560 },	/* 1.2 V */
+};
+
 static const struct msm_dsi_config sdm660_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 1,
-		.regs = {
-			{"vdda", 12560},	/* 1.2 V */
-		},
-	},
+	.regulator_data = sdm660_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(sdm660_dsi_regulators),
 	.bus_clk_names = dsi_sdm660_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_sdm660_bus_clk_names),
 	.io_start = { 0xc994000, 0xc996000 },
@@ -167,28 +167,28 @@ static const char * const dsi_sc7180_bus_clk_names[] = {
 	"iface", "bus",
 };
 
+static const struct regulator_bulk_data sdm845_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 21800 },	/* 1.2 V */
+};
+
 static const struct msm_dsi_config sdm845_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 1,
-		.regs = {
-			{"vdda", 21800},	/* 1.2 V */
-		},
-	},
+	.regulator_data = sdm845_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(sdm845_dsi_regulators),
 	.bus_clk_names = dsi_sdm845_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_sdm845_bus_clk_names),
 	.io_start = { 0xae94000, 0xae96000 },
 	.num_dsi = 2,
 };
 
+static const struct regulator_bulk_data sc7180_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 21800 },	/* 1.2 V */
+};
+
 static const struct msm_dsi_config sc7180_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 1,
-		.regs = {
-			{"vdda", 21800},	/* 1.2 V */
-		},
-	},
+	.regulator_data = sc7180_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(sc7180_dsi_regulators),
 	.bus_clk_names = dsi_sc7180_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_sc7180_bus_clk_names),
 	.io_start = { 0xae94000 },
@@ -199,14 +199,14 @@ static const char * const dsi_sc7280_bus_clk_names[] = {
 	"iface", "bus",
 };
 
+static const struct regulator_bulk_data sc7280_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 8350 },	/* 1.2 V */
+};
+
 static const struct msm_dsi_config sc7280_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 1,
-		.regs = {
-			{"vdda", 8350},	/* 1.2 V */
-		},
-	},
+	.regulator_data = sc7280_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(sc7280_dsi_regulators),
 	.bus_clk_names = dsi_sc7280_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_sc7280_bus_clk_names),
 	.io_start = { 0xae94000 },
@@ -217,14 +217,14 @@ static const char * const dsi_qcm2290_bus_clk_names[] = {
 	"iface", "bus",
 };
 
+static const struct regulator_bulk_data qcm2290_dsi_cfg_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 21800 },	/* 1.2 V */
+};
+
 static const struct msm_dsi_config qcm2290_dsi_cfg = {
 	.io_offset = DSI_6G_REG_SHIFT,
-	.reg_cfg = {
-		.num = 1,
-		.regs = {
-			{"vdda", 21800},	/* 1.2 V */
-		},
-	},
+	.regulator_data = qcm2290_dsi_cfg_regulators,
+	.num_regulators = ARRAY_SIZE(qcm2290_dsi_cfg_regulators),
 	.bus_clk_names = dsi_qcm2290_bus_clk_names,
 	.num_bus_clks = ARRAY_SIZE(dsi_qcm2290_bus_clk_names),
 	.io_start = { 0x5e94000 },
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
index fe54a999968b..8f04e685a74e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
@@ -32,7 +32,8 @@
 
 struct msm_dsi_config {
 	u32 io_offset;
-	struct dsi_reg_config reg_cfg;
+	const struct regulator_bulk_data *regulator_data;
+	int num_regulators;
 	const char * const *bus_clk_names;
 	const int num_bus_clks;
 	const resource_size_t io_start[DSI_MAX];
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index dec7a94cf819..e159f2006158 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -106,7 +106,7 @@ struct msm_dsi_host {
 
 	void __iomem *ctrl_base;
 	phys_addr_t ctrl_size;
-	struct regulator_bulk_data supplies[DSI_DEV_REGULATOR_MAX];
+	struct regulator_bulk_data *supplies;
 
 	int num_bus_clks;
 	struct clk_bulk_data bus_clks[DSI_BUS_CLK_MAX];
@@ -259,7 +259,7 @@ static inline struct msm_dsi_host *to_msm_dsi_host(struct mipi_dsi_host *host)
 static void dsi_host_regulator_disable(struct msm_dsi_host *msm_host)
 {
 	struct regulator_bulk_data *s = msm_host->supplies;
-	int num = msm_host->cfg_hnd->cfg->reg_cfg.num;
+	int num = msm_host->cfg_hnd->cfg->num_regulators;
 
 	regulator_bulk_disable(num, s);
 }
@@ -267,31 +267,16 @@ static void dsi_host_regulator_disable(struct msm_dsi_host *msm_host)
 static int dsi_host_regulator_enable(struct msm_dsi_host *msm_host)
 {
 	struct regulator_bulk_data *s = msm_host->supplies;
-	int num = msm_host->cfg_hnd->cfg->reg_cfg.num;
+	int num = msm_host->cfg_hnd->cfg->num_regulators;
 
 	return regulator_bulk_enable(num, s);
 }
 
 static int dsi_regulator_init(struct msm_dsi_host *msm_host)
 {
-	struct regulator_bulk_data *s = msm_host->supplies;
-	const struct dsi_reg_entry *regs = msm_host->cfg_hnd->cfg->reg_cfg.regs;
-	int num = msm_host->cfg_hnd->cfg->reg_cfg.num;
-	int i, ret;
-
-	for (i = 0; i < num; i++) {
-		s[i].supply = regs[i].name;
-		s[i].init_load_uA = regs[i].enable_load;
-	}
-
-	ret = devm_regulator_bulk_get(&msm_host->pdev->dev, num, s);
-	if (ret < 0) {
-		pr_err("%s: failed to init regulator, ret=%d\n",
-						__func__, ret);
-		return ret;
-	}
-
-	return 0;
+	return devm_regulator_bulk_get_const(
+		&msm_host->pdev->dev, msm_host->cfg_hnd->cfg->num_regulators,
+		msm_host->cfg_hnd->cfg->regulator_data, &msm_host->supplies);
 }
 
 int dsi_clk_init_v2(struct msm_dsi_host *msm_host)
-- 
2.37.1.359.gd136c6c3e2-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: (subset) [PATCH v2 0/7] drm/msm/dsi regulator improvements
  2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
                   ` (6 preceding siblings ...)
  2022-07-26 17:38 ` [PATCH v2 7/7] drm/msm/dsi: Take advantage of devm_regulator_bulk_get_const() Douglas Anderson
@ 2022-07-27 23:02 ` Mark Brown
  2022-07-28  0:20 ` Mark Brown
  8 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2022-07-27 23:02 UTC (permalink / raw)
  To: Abhinav Kumar, Douglas Anderson, Rob Clark, Dmitry Baryshkov
  Cc: linux-arm-msm, freedreno, Loic Poulain, Jonathan Marek,
	David Airlie, Rajeev Nandan, Konrad Dybcio, Vladimir Lypak,
	Liam Girdwood, dri-devel, Stephen Boyd, Vinod Koul,
	José Expósito, AngeloGioacchino Del Regno,
	Marijn Suijten, Bjorn Andersson, Sean Paul, linux-kernel

On Tue, 26 Jul 2022 10:38:17 -0700, Douglas Anderson wrote:
> The main goal of this series is to make a small dent in cleaning up
> the way we deal with regulator loads. The idea is to add some extra
> functionality to the regulator "bulk" API so that consumers can
> specify the load using that. Though I didn't convert everyone over, I
> include patches in this series that show how the Qualcomm DSI driver
> is improved by this.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[4/7] regulator: core: Allow specifying an initial load w/ the bulk API
      commit: 6eabfc018e8d1033e7fc1efce30a872e2dccb537
[6/7] regulator: core: Allow drivers to define their init data as const
      commit: 1de452a0edda26f1483d1d934f692eab13ba669a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/7] drm/msm/dsi regulator improvements
  2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
                   ` (7 preceding siblings ...)
  2022-07-27 23:02 ` (subset) [PATCH v2 0/7] drm/msm/dsi regulator improvements Mark Brown
@ 2022-07-28  0:20 ` Mark Brown
  8 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2022-07-28  0:20 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: David Airlie, Konrad Dybcio, dri-devel, Bjorn Andersson,
	AngeloGioacchino Del Regno, Marijn Suijten, Archit Taneja,
	Jonathan Marek, Rajeev Nandan, linux-arm-msm, Abhinav Kumar,
	Stephen Boyd, Sean Paul, Loic Poulain, Vladimir Lypak,
	Liam Girdwood, linux-kernel, Vinod Koul, Dmitry Baryshkov,
	José Expósito, freedreno

[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]

On Tue, Jul 26, 2022 at 10:38:17AM -0700, Douglas Anderson wrote:

> * After that I have patches that add to the regulator API and then
>   show a usage of those in the DSI driver. I'd expect that the two
>   regulator patches could land in the regulator tree. The DSI patches
>   would need to wait until the new regulator changes are available.

The following changes since commit f2906aa863381afb0015a9eb7fefad885d4e5a56:

  Linux 5.19-rc1 (2022-06-05 17:18:54 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git tags/regulator-load-bulk-api

for you to fetch changes up to 1de452a0edda26f1483d1d934f692eab13ba669a:

  regulator: core: Allow drivers to define their init data as const (2022-07-27 13:47:30 +0100)

----------------------------------------------------------------
regulator: Consumer load management improvements

The main goal of this series is to make a small dent in cleaning up
the way we deal with regulator loads. The idea is to add some extra
functionality to the regulator "bulk" API so that consumers can
specify the load using that.

----------------------------------------------------------------
Douglas Anderson (2):
      regulator: core: Allow specifying an initial load w/ the bulk API
      regulator: core: Allow drivers to define their init data as const

 drivers/regulator/core.c           | 20 ++++++++++++--------
 drivers/regulator/devres.c         | 28 ++++++++++++++++++++++++++++
 include/linux/regulator/consumer.h | 16 ++++++++++++----
 3 files changed, 52 insertions(+), 12 deletions(-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API
  2022-07-26 17:38 ` [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API Douglas Anderson
@ 2022-08-16 12:58   ` Yongqin Liu
  2022-08-17 14:25     ` Doug Anderson
  0 siblings, 1 reply; 16+ messages in thread
From: Yongqin Liu @ 2022-08-16 12:58 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: linux-arm-msm, Abhinav Kumar, dri-devel, Liam Girdwood,
	Mark Brown, John Stultz, Dmitry Baryshkov, freedreno,
	Sumit Semwal, linux-kernel

HI, Douglas

With this change, I get one kernel panic with my hikey960
android-mainline based Android build,
if it's reverted, then the build could boot to the home screen successfully.
From the log information I shared here, not sure if you have any idea
what I could do to have the hikey960
build work with this change,

The kernel panic is something like this, for details, please check the
log here: http://ix.io/47Lq

[   10.738042][  T193] adv7511 1-0039: error 0000000000000000: Failed
to get supply 'v1p2'
[   10.748457][  T194] apexd: Found pre-installed APEX
/system/apex/com.android.os.statsd.apex
[   10.752908][   T67] Unable to handle kernel read from unreadable
memory at virtual address 000000000000004c
[   10.753116][    T8] Unable to handle kernel read from unreadable
memory at virtual address 0000000000000078
[   10.753130][    T8] Mem abort info:
[   10.753135][    T8]   ESR = 0x0000000096000005
[   10.753142][    T8]   EC = 0x25: DABT (current EL), IL = 32 bits
[   10.753152][    T8]   SET = 0, FnV = 0
[   10.753159][    T8]   EA = 0, S1PTW = 0
[   10.753166][    T8]   FSC = 0x05: level 1 translation fault
[   10.753174][    T8] Data abort info:
[   10.753179][    T8]   ISV = 0, ISS = 0x00000005
[   10.753184][    T8]   CM = 0, WnR = 0
[   10.753192][    T8] user pgtable: 4k pages, 39-bit VAs, pgdp=0000000003098000
[   10.753204][    T8] [0000000000000078] pgd=0000000000000000,
p4d=0000000000000000, pud=0000000000000000
[   10.753232][    T8] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[   10.753245][    T8] Modules linked in: adv7511(E+) tcpci_rt1711h(E)
hci_uart(E) btqca(E) btbcm(E) cpufreq_dt(E) hi3660_i2s(E)
hisi_hikey_usb(E) hi6421_pmic_core(E) syscon_reboot_mode(E)
reboot_mode(E) hi3660_mailbox(E) dw_mmc_k3(E) hisi_thermal(E)
dw_mmc_pltfm(E) dw_mmc(E) kirin_drm(E) snd_soc_simple_card(E)
snd_soc_simple_card_utils(E) spi_pl022(E) kirin_dsi(E)
phy_hi3660_usb3(E) i2c_designware_platform(E) drm_cma_helper(E)
i2c_designware_core(E) mali_kbase(OE) k3dma(E) cma_heap(E)
system_heap(E)
[   10.753436][    T8] CPU: 2 PID: 8 Comm: kworker/u16:0 Tainted: G
       OE      5.19.0-mainline-03487-g86d047950300-dirty #1
[   10.753454][    T8] Hardware name: HiKey960 (DT)
[   10.753463][    T8] Workqueue: events_unbound async_run_entry_fn
[   10.753497][    T8] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT
-SSBS BTYPE=--)
[   10.753516][    T8] pc : regulator_bulk_enable_async+0x3c/0x98
[   10.753540][    T8] lr : async_run_entry_fn+0x30/0xf8
[   10.753559][    T8] sp : ffffffc009ed3d20
[   10.753567][    T8] x29: ffffffc009ed3d40 x28: 0000000000000402
x27: ffffff801ad99828
[   10.753592][    T8] x26: ffffff803217b010 x25: 0000000000000002
x24: ffffff8003385da8
[   10.753617][    T8] x23: ffffff8003385da0 x22: ffffff801ad94805
x21: ffffff8003385da0
[   10.753642][    T8] x20: 0000000000000000 x19: ffffff8003143d20
x18: ffffffc008075028
[   10.753667][    T8] x17: 000000040044ffff x16: 0000000000000001
x15: 0000000000000010
[   10.753691][    T8] x14: 0000000000000000 x13: 0000000000000f58
x12: 0000000082355555
[   10.753715][    T8] x11: 00006acfbfa2f400 x10: 0000000000000016 x9
: 00ffffffffffffff
[   10.753740][    T8] x8 : da9ecda1b63b0500 x7 : 0000000000008080 x6
: 0000000000000000
[   10.753764][    T8] x5 : 0000000000000001 x4 : 0000646e756f626e x3
: ffffff801ad99828
[   10.753788][    T8] x2 : ffffff8003385da8 x1 : ffffffc009ed3d20 x0
: ffffff8003143d20
[   10.753812][    T8] Call trace:
[   10.753818][    T8]  regulator_bulk_enable_async+0x3c/0x98
[   10.753839][    T8]  async_run_entry_fn+0x30/0xf8
[   10.753859][    T8]  process_one_work+0x1d0/0x404
[   10.753879][    T8]  worker_thread+0x25c/0x43c
[   10.753897][    T8]  kthread+0xf0/0x2c0
[   10.753912][    T8]  ret_from_fork+0x10/0x20
[   10.753940][    T8] Code: f81f83a8 f9400814 a900ffff f90003ff (f9403e95)
[   10.753950][    T8] ---[ end trace 0000000000000000 ]---
[   10.760621][  T194] apexd: Found pre-installed APEX
/system/apex/com.android.permission.capex
[   10.767672][   T67] Mem abort info:
[   10.779658][  T194] apexd: Found pre-installed APEX
/system/apex/com.android.art.capex
[   10.783690][   T67]   ESR = 0x0000000096000005
[   10.792424][  T194] apexd: Found pre-installed APEX
/system/apex/com.android.scheduling.capex
[   10.794713][    T8] Kernel panic - not syncing: Oops: Fatal exception
[   10.794723][    T8] SMP: stopping secondary CPUs
[   10.798141][    T8] Kernel Offset: 0x70000 from 0xffffffc008000000
[   10.798150][    T8] PHYS_OFFSET: 0x0
[   10.798156][    T8] CPU features: 0x0000,00649020,00001086
[   10.798167][    T8] Memory Limit: none

Thanks,
Yongqin Liu

On Wed, 27 Jul 2022 at 01:39, Douglas Anderson <dianders@chromium.org> wrote:
>
> There are a number of drivers that follow a pattern that looks like
> this:
> 1. Use the regulator bulk API to get a bunch of regulators.
> 2. Set the load on each of the regulators to use whenever the
>    regulators are enabled.
>
> Let's make this easier by just allowing the drivers to pass the load
> in.
>
> As part of this change we need to move the error printing in
> regulator_bulk_get() around; let's switch to the new dev_err_probe()
> to simplify it.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - ("Allow specifying an initial load w/ the bulk API") new for v2.
>
>  drivers/regulator/core.c           | 20 ++++++++++++--------
>  include/linux/regulator/consumer.h | 12 ++++++++----
>  2 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 1e54a833f2cf..17c476fc8adb 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -4783,22 +4783,26 @@ int regulator_bulk_get(struct device *dev, int num_consumers,
>                 consumers[i].consumer = regulator_get(dev,
>                                                       consumers[i].supply);
>                 if (IS_ERR(consumers[i].consumer)) {
> -                       ret = PTR_ERR(consumers[i].consumer);
>                         consumers[i].consumer = NULL;
> +                       ret = dev_err_probe(dev, PTR_ERR(consumers[i].consumer),
> +                                           "Failed to get supply '%s'",
> +                                           consumers[i].supply);
>                         goto err;
>                 }
> +
> +               if (consumers[i].init_load_uA > 0) {
> +                       ret = regulator_set_load(consumers[i].consumer,
> +                                                consumers[i].init_load_uA);
> +                       if (ret) {
> +                               i++;
> +                               goto err;
> +                       }
> +               }
>         }
>
>         return 0;
>
>  err:
> -       if (ret != -EPROBE_DEFER)
> -               dev_err(dev, "Failed to get supply '%s': %pe\n",
> -                       consumers[i].supply, ERR_PTR(ret));
> -       else
> -               dev_dbg(dev, "Failed to get supply '%s', deferring\n",
> -                       consumers[i].supply);
> -
>         while (--i >= 0)
>                 regulator_put(consumers[i].consumer);
>
> diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
> index bbf6590a6dec..5779f4466e62 100644
> --- a/include/linux/regulator/consumer.h
> +++ b/include/linux/regulator/consumer.h
> @@ -171,10 +171,13 @@ struct regulator;
>  /**
>   * struct regulator_bulk_data - Data used for bulk regulator operations.
>   *
> - * @supply:   The name of the supply.  Initialised by the user before
> - *            using the bulk regulator APIs.
> - * @consumer: The regulator consumer for the supply.  This will be managed
> - *            by the bulk API.
> + * @supply:       The name of the supply.  Initialised by the user before
> + *                using the bulk regulator APIs.
> + * @init_load_uA: After getting the regulator, regulator_set_load() will be
> + *                called with this load.  Initialised by the user before
> + *                using the bulk regulator APIs.
> + * @consumer:     The regulator consumer for the supply.  This will be managed
> + *                by the bulk API.
>   *
>   * The regulator APIs provide a series of regulator_bulk_() API calls as
>   * a convenience to consumers which require multiple supplies.  This
> @@ -182,6 +185,7 @@ struct regulator;
>   */
>  struct regulator_bulk_data {
>         const char *supply;
> +       int init_load_uA;
>         struct regulator *consumer;
>
>         /* private: Internal use */
> --
> 2.37.1.359.gd136c6c3e2-goog
>


-- 
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API
  2022-08-16 12:58   ` Yongqin Liu
@ 2022-08-17 14:25     ` Doug Anderson
  2022-08-17 16:52       ` Yongqin Liu
  0 siblings, 1 reply; 16+ messages in thread
From: Doug Anderson @ 2022-08-17 14:25 UTC (permalink / raw)
  To: Yongqin Liu
  Cc: linux-arm-msm, Abhinav Kumar, dri-devel, Liam Girdwood,
	Mark Brown, John Stultz, Dmitry Baryshkov, freedreno,
	Sumit Semwal, LKML

Hi,

On Tue, Aug 16, 2022 at 5:58 AM Yongqin Liu <yongqin.liu@linaro.org> wrote:
>
> HI, Douglas
>
> With this change, I get one kernel panic with my hikey960
> android-mainline based Android build,
> if it's reverted, then the build could boot to the home screen successfully.
> From the log information I shared here, not sure if you have any idea
> what I could do to have the hikey960
> build work with this change,
>
> The kernel panic is something like this, for details, please check the
> log here: http://ix.io/47Lq
>
> [   10.738042][  T193] adv7511 1-0039: error 0000000000000000: Failed
> to get supply 'v1p2'
> [   10.748457][  T194] apexd: Found pre-installed APEX
> /system/apex/com.android.os.statsd.apex
> [   10.752908][   T67] Unable to handle kernel read from unreadable
> memory at virtual address 000000000000004c
> [   10.753116][    T8] Unable to handle kernel read from unreadable
> memory at virtual address 0000000000000078
> [   10.753130][    T8] Mem abort info:
> [   10.753135][    T8]   ESR = 0x0000000096000005
> [   10.753142][    T8]   EC = 0x25: DABT (current EL), IL = 32 bits
> [   10.753152][    T8]   SET = 0, FnV = 0
> [   10.753159][    T8]   EA = 0, S1PTW = 0
> [   10.753166][    T8]   FSC = 0x05: level 1 translation fault
> [   10.753174][    T8] Data abort info:
> [   10.753179][    T8]   ISV = 0, ISS = 0x00000005
> [   10.753184][    T8]   CM = 0, WnR = 0
> [   10.753192][    T8] user pgtable: 4k pages, 39-bit VAs, pgdp=0000000003098000
> [   10.753204][    T8] [0000000000000078] pgd=0000000000000000,
> p4d=0000000000000000, pud=0000000000000000
> [   10.753232][    T8] Internal error: Oops: 96000005 [#1] PREEMPT SMP
> [   10.753245][    T8] Modules linked in: adv7511(E+) tcpci_rt1711h(E)
> hci_uart(E) btqca(E) btbcm(E) cpufreq_dt(E) hi3660_i2s(E)
> hisi_hikey_usb(E) hi6421_pmic_core(E) syscon_reboot_mode(E)
> reboot_mode(E) hi3660_mailbox(E) dw_mmc_k3(E) hisi_thermal(E)
> dw_mmc_pltfm(E) dw_mmc(E) kirin_drm(E) snd_soc_simple_card(E)
> snd_soc_simple_card_utils(E) spi_pl022(E) kirin_dsi(E)
> phy_hi3660_usb3(E) i2c_designware_platform(E) drm_cma_helper(E)
> i2c_designware_core(E) mali_kbase(OE) k3dma(E) cma_heap(E)
> system_heap(E)
> [   10.753436][    T8] CPU: 2 PID: 8 Comm: kworker/u16:0 Tainted: G
>        OE      5.19.0-mainline-03487-g86d047950300-dirty #1
> [   10.753454][    T8] Hardware name: HiKey960 (DT)
> [   10.753463][    T8] Workqueue: events_unbound async_run_entry_fn
> [   10.753497][    T8] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT
> -SSBS BTYPE=--)
> [   10.753516][    T8] pc : regulator_bulk_enable_async+0x3c/0x98
> [   10.753540][    T8] lr : async_run_entry_fn+0x30/0xf8
> [   10.753559][    T8] sp : ffffffc009ed3d20
> [   10.753567][    T8] x29: ffffffc009ed3d40 x28: 0000000000000402
> x27: ffffff801ad99828
> [   10.753592][    T8] x26: ffffff803217b010 x25: 0000000000000002
> x24: ffffff8003385da8
> [   10.753617][    T8] x23: ffffff8003385da0 x22: ffffff801ad94805
> x21: ffffff8003385da0
> [   10.753642][    T8] x20: 0000000000000000 x19: ffffff8003143d20
> x18: ffffffc008075028
> [   10.753667][    T8] x17: 000000040044ffff x16: 0000000000000001
> x15: 0000000000000010
> [   10.753691][    T8] x14: 0000000000000000 x13: 0000000000000f58
> x12: 0000000082355555
> [   10.753715][    T8] x11: 00006acfbfa2f400 x10: 0000000000000016 x9
> : 00ffffffffffffff
> [   10.753740][    T8] x8 : da9ecda1b63b0500 x7 : 0000000000008080 x6
> : 0000000000000000
> [   10.753764][    T8] x5 : 0000000000000001 x4 : 0000646e756f626e x3
> : ffffff801ad99828
> [   10.753788][    T8] x2 : ffffff8003385da8 x1 : ffffffc009ed3d20 x0
> : ffffff8003143d20
> [   10.753812][    T8] Call trace:
> [   10.753818][    T8]  regulator_bulk_enable_async+0x3c/0x98
> [   10.753839][    T8]  async_run_entry_fn+0x30/0xf8
> [   10.753859][    T8]  process_one_work+0x1d0/0x404
> [   10.753879][    T8]  worker_thread+0x25c/0x43c
> [   10.753897][    T8]  kthread+0xf0/0x2c0
> [   10.753912][    T8]  ret_from_fork+0x10/0x20
> [   10.753940][    T8] Code: f81f83a8 f9400814 a900ffff f90003ff (f9403e95)
> [   10.753950][    T8] ---[ end trace 0000000000000000 ]---
> [   10.760621][  T194] apexd: Found pre-installed APEX
> /system/apex/com.android.permission.capex
> [   10.767672][   T67] Mem abort info:
> [   10.779658][  T194] apexd: Found pre-installed APEX
> /system/apex/com.android.art.capex
> [   10.783690][   T67]   ESR = 0x0000000096000005
> [   10.792424][  T194] apexd: Found pre-installed APEX
> /system/apex/com.android.scheduling.capex
> [   10.794713][    T8] Kernel panic - not syncing: Oops: Fatal exception
> [   10.794723][    T8] SMP: stopping secondary CPUs
> [   10.798141][    T8] Kernel Offset: 0x70000 from 0xffffffc008000000
> [   10.798150][    T8] PHYS_OFFSET: 0x0
> [   10.798156][    T8] CPU features: 0x0000,00649020,00001086
> [   10.798167][    T8] Memory Limit: none

Are you fixed by the patch ("regulator: core: Fix missing error return
from regulator_bulk_get()") [1]

[1] https://lore.kernel.org/r/20220809142738.1.I91625242f137c707bb345c51c80c5ecee02eeff3@changeid

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API
  2022-08-17 14:25     ` Doug Anderson
@ 2022-08-17 16:52       ` Yongqin Liu
  2022-08-23  6:22         ` Yongqin Liu
  0 siblings, 1 reply; 16+ messages in thread
From: Yongqin Liu @ 2022-08-17 16:52 UTC (permalink / raw)
  To: Doug Anderson
  Cc: linux-arm-msm, Abhinav Kumar, dri-devel, Liam Girdwood,
	Mark Brown, John Stultz, Dmitry Baryshkov, freedreno,
	Sumit Semwal, LKML

Hi, Douglas

On Wed, 17 Aug 2022 at 22:26, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Aug 16, 2022 at 5:58 AM Yongqin Liu <yongqin.liu@linaro.org> wrote:
> >
> > HI, Douglas
> >
> > With this change, I get one kernel panic with my hikey960
> > android-mainline based Android build,
> > if it's reverted, then the build could boot to the home screen successfully.
> > From the log information I shared here, not sure if you have any idea
> > what I could do to have the hikey960
> > build work with this change,
> >
> > The kernel panic is something like this, for details, please check the
> > log here: http://ix.io/47Lq
> >
> > [   10.738042][  T193] adv7511 1-0039: error 0000000000000000: Failed
> > to get supply 'v1p2'
> > [   10.748457][  T194] apexd: Found pre-installed APEX
> > /system/apex/com.android.os.statsd.apex
> > [   10.752908][   T67] Unable to handle kernel read from unreadable
> > memory at virtual address 000000000000004c
> > [   10.753116][    T8] Unable to handle kernel read from unreadable
> > memory at virtual address 0000000000000078
> > [   10.753130][    T8] Mem abort info:
> > [   10.753135][    T8]   ESR = 0x0000000096000005
> > [   10.753142][    T8]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [   10.753152][    T8]   SET = 0, FnV = 0
> > [   10.753159][    T8]   EA = 0, S1PTW = 0
> > [   10.753166][    T8]   FSC = 0x05: level 1 translation fault
> > [   10.753174][    T8] Data abort info:
> > [   10.753179][    T8]   ISV = 0, ISS = 0x00000005
> > [   10.753184][    T8]   CM = 0, WnR = 0
> > [   10.753192][    T8] user pgtable: 4k pages, 39-bit VAs, pgdp=0000000003098000
> > [   10.753204][    T8] [0000000000000078] pgd=0000000000000000,
> > p4d=0000000000000000, pud=0000000000000000
> > [   10.753232][    T8] Internal error: Oops: 96000005 [#1] PREEMPT SMP
> > [   10.753245][    T8] Modules linked in: adv7511(E+) tcpci_rt1711h(E)
> > hci_uart(E) btqca(E) btbcm(E) cpufreq_dt(E) hi3660_i2s(E)
> > hisi_hikey_usb(E) hi6421_pmic_core(E) syscon_reboot_mode(E)
> > reboot_mode(E) hi3660_mailbox(E) dw_mmc_k3(E) hisi_thermal(E)
> > dw_mmc_pltfm(E) dw_mmc(E) kirin_drm(E) snd_soc_simple_card(E)
> > snd_soc_simple_card_utils(E) spi_pl022(E) kirin_dsi(E)
> > phy_hi3660_usb3(E) i2c_designware_platform(E) drm_cma_helper(E)
> > i2c_designware_core(E) mali_kbase(OE) k3dma(E) cma_heap(E)
> > system_heap(E)
> > [   10.753436][    T8] CPU: 2 PID: 8 Comm: kworker/u16:0 Tainted: G
> >        OE      5.19.0-mainline-03487-g86d047950300-dirty #1
> > [   10.753454][    T8] Hardware name: HiKey960 (DT)
> > [   10.753463][    T8] Workqueue: events_unbound async_run_entry_fn
> > [   10.753497][    T8] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT
> > -SSBS BTYPE=--)
> > [   10.753516][    T8] pc : regulator_bulk_enable_async+0x3c/0x98
> > [   10.753540][    T8] lr : async_run_entry_fn+0x30/0xf8
> > [   10.753559][    T8] sp : ffffffc009ed3d20
> > [   10.753567][    T8] x29: ffffffc009ed3d40 x28: 0000000000000402
> > x27: ffffff801ad99828
> > [   10.753592][    T8] x26: ffffff803217b010 x25: 0000000000000002
> > x24: ffffff8003385da8
> > [   10.753617][    T8] x23: ffffff8003385da0 x22: ffffff801ad94805
> > x21: ffffff8003385da0
> > [   10.753642][    T8] x20: 0000000000000000 x19: ffffff8003143d20
> > x18: ffffffc008075028
> > [   10.753667][    T8] x17: 000000040044ffff x16: 0000000000000001
> > x15: 0000000000000010
> > [   10.753691][    T8] x14: 0000000000000000 x13: 0000000000000f58
> > x12: 0000000082355555
> > [   10.753715][    T8] x11: 00006acfbfa2f400 x10: 0000000000000016 x9
> > : 00ffffffffffffff
> > [   10.753740][    T8] x8 : da9ecda1b63b0500 x7 : 0000000000008080 x6
> > : 0000000000000000
> > [   10.753764][    T8] x5 : 0000000000000001 x4 : 0000646e756f626e x3
> > : ffffff801ad99828
> > [   10.753788][    T8] x2 : ffffff8003385da8 x1 : ffffffc009ed3d20 x0
> > : ffffff8003143d20
> > [   10.753812][    T8] Call trace:
> > [   10.753818][    T8]  regulator_bulk_enable_async+0x3c/0x98
> > [   10.753839][    T8]  async_run_entry_fn+0x30/0xf8
> > [   10.753859][    T8]  process_one_work+0x1d0/0x404
> > [   10.753879][    T8]  worker_thread+0x25c/0x43c
> > [   10.753897][    T8]  kthread+0xf0/0x2c0
> > [   10.753912][    T8]  ret_from_fork+0x10/0x20
> > [   10.753940][    T8] Code: f81f83a8 f9400814 a900ffff f90003ff (f9403e95)
> > [   10.753950][    T8] ---[ end trace 0000000000000000 ]---
> > [   10.760621][  T194] apexd: Found pre-installed APEX
> > /system/apex/com.android.permission.capex
> > [   10.767672][   T67] Mem abort info:
> > [   10.779658][  T194] apexd: Found pre-installed APEX
> > /system/apex/com.android.art.capex
> > [   10.783690][   T67]   ESR = 0x0000000096000005
> > [   10.792424][  T194] apexd: Found pre-installed APEX
> > /system/apex/com.android.scheduling.capex
> > [   10.794713][    T8] Kernel panic - not syncing: Oops: Fatal exception
> > [   10.794723][    T8] SMP: stopping secondary CPUs
> > [   10.798141][    T8] Kernel Offset: 0x70000 from 0xffffffc008000000
> > [   10.798150][    T8] PHYS_OFFSET: 0x0
> > [   10.798156][    T8] CPU features: 0x0000,00649020,00001086
> > [   10.798167][    T8] Memory Limit: none
>
> Are you fixed by the patch ("regulator: core: Fix missing error return
> from regulator_bulk_get()") [1]
>
> [1] https://lore.kernel.org/r/20220809142738.1.I91625242f137c707bb345c51c80c5ecee02eeff3@changeid

Thanks for the check!
I tested the above patch with the android-mainline based kernel,
and it fixes the regulator_bulk_enable_async kernel panic reported here.

-- 
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API
  2022-08-17 16:52       ` Yongqin Liu
@ 2022-08-23  6:22         ` Yongqin Liu
  2022-08-23 14:50           ` Doug Anderson
  0 siblings, 1 reply; 16+ messages in thread
From: Yongqin Liu @ 2022-08-23  6:22 UTC (permalink / raw)
  To: Doug Anderson
  Cc: linux-arm-msm, Abhinav Kumar, dri-devel, Liam Girdwood,
	Steve Muckle, Alistair Delva, Mark Brown, John Stultz,
	Dmitry Baryshkov, freedreno, Sumit Semwal, LKML, Todd Kjos

Hi, Douglas

Just an update on the fix you pointed out previously here:
> > [1] https://lore.kernel.org/r/20220809142738.1.I91625242f137c707bb345c51c80c5ecee02eeff3@changeid

With it I could boot the hikey960 build to the home screen if it does
not use the GKI kernel.
but the problem will be reproduced if it uses the GKI kernel.

And if this change is reverted, then it could boot with the GKI kernel as well.

I am not sure what's the reason there, but there seems to be some
difference with the fix above and the workaround of revert.
Not sure if you have any idea about that.

Regarding the GKI kernel(Android Generic Kernel Image)[2], it's built
from the android-mainline tree(f51334eac4de) without any workaround.
(Neither the revert, nor the fix applied), and the regulator modules
used for the hikey960 build are hi6421v530-regulator.ko and
hi655x-regulator.ko

I am still not sure if it would work with the GKI kernel that has the
fix that you pointed out in. the case that both the GKI kernel and
vendor tree have the fix.
Will update here when I have some results.

[2]: https://source.android.com/docs/core/architecture/kernel/generic-kernel-image?hl=en

Thanks,
Yongqin Liu

On Thu, 18 Aug 2022 at 00:52, Yongqin Liu <yongqin.liu@linaro.org> wrote:
>
> Hi, Douglas
>
> On Wed, 17 Aug 2022 at 22:26, Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Tue, Aug 16, 2022 at 5:58 AM Yongqin Liu <yongqin.liu@linaro.org> wrote:
> > >
> > > HI, Douglas
> > >
> > > With this change, I get one kernel panic with my hikey960
> > > android-mainline based Android build,
> > > if it's reverted, then the build could boot to the home screen successfully.
> > > From the log information I shared here, not sure if you have any idea
> > > what I could do to have the hikey960
> > > build work with this change,
> > >
> > > The kernel panic is something like this, for details, please check the
> > > log here: http://ix.io/47Lq
> > >
> > > [   10.738042][  T193] adv7511 1-0039: error 0000000000000000: Failed
> > > to get supply 'v1p2'
> > > [   10.748457][  T194] apexd: Found pre-installed APEX
> > > /system/apex/com.android.os.statsd.apex
> > > [   10.752908][   T67] Unable to handle kernel read from unreadable
> > > memory at virtual address 000000000000004c
> > > [   10.753116][    T8] Unable to handle kernel read from unreadable
> > > memory at virtual address 0000000000000078
> > > [   10.753130][    T8] Mem abort info:
> > > [   10.753135][    T8]   ESR = 0x0000000096000005
> > > [   10.753142][    T8]   EC = 0x25: DABT (current EL), IL = 32 bits
> > > [   10.753152][    T8]   SET = 0, FnV = 0
> > > [   10.753159][    T8]   EA = 0, S1PTW = 0
> > > [   10.753166][    T8]   FSC = 0x05: level 1 translation fault
> > > [   10.753174][    T8] Data abort info:
> > > [   10.753179][    T8]   ISV = 0, ISS = 0x00000005
> > > [   10.753184][    T8]   CM = 0, WnR = 0
> > > [   10.753192][    T8] user pgtable: 4k pages, 39-bit VAs, pgdp=0000000003098000
> > > [   10.753204][    T8] [0000000000000078] pgd=0000000000000000,
> > > p4d=0000000000000000, pud=0000000000000000
> > > [   10.753232][    T8] Internal error: Oops: 96000005 [#1] PREEMPT SMP
> > > [   10.753245][    T8] Modules linked in: adv7511(E+) tcpci_rt1711h(E)
> > > hci_uart(E) btqca(E) btbcm(E) cpufreq_dt(E) hi3660_i2s(E)
> > > hisi_hikey_usb(E) hi6421_pmic_core(E) syscon_reboot_mode(E)
> > > reboot_mode(E) hi3660_mailbox(E) dw_mmc_k3(E) hisi_thermal(E)
> > > dw_mmc_pltfm(E) dw_mmc(E) kirin_drm(E) snd_soc_simple_card(E)
> > > snd_soc_simple_card_utils(E) spi_pl022(E) kirin_dsi(E)
> > > phy_hi3660_usb3(E) i2c_designware_platform(E) drm_cma_helper(E)
> > > i2c_designware_core(E) mali_kbase(OE) k3dma(E) cma_heap(E)
> > > system_heap(E)
> > > [   10.753436][    T8] CPU: 2 PID: 8 Comm: kworker/u16:0 Tainted: G
> > >        OE      5.19.0-mainline-03487-g86d047950300-dirty #1
> > > [   10.753454][    T8] Hardware name: HiKey960 (DT)
> > > [   10.753463][    T8] Workqueue: events_unbound async_run_entry_fn
> > > [   10.753497][    T8] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT
> > > -SSBS BTYPE=--)
> > > [   10.753516][    T8] pc : regulator_bulk_enable_async+0x3c/0x98
> > > [   10.753540][    T8] lr : async_run_entry_fn+0x30/0xf8
> > > [   10.753559][    T8] sp : ffffffc009ed3d20
> > > [   10.753567][    T8] x29: ffffffc009ed3d40 x28: 0000000000000402
> > > x27: ffffff801ad99828
> > > [   10.753592][    T8] x26: ffffff803217b010 x25: 0000000000000002
> > > x24: ffffff8003385da8
> > > [   10.753617][    T8] x23: ffffff8003385da0 x22: ffffff801ad94805
> > > x21: ffffff8003385da0
> > > [   10.753642][    T8] x20: 0000000000000000 x19: ffffff8003143d20
> > > x18: ffffffc008075028
> > > [   10.753667][    T8] x17: 000000040044ffff x16: 0000000000000001
> > > x15: 0000000000000010
> > > [   10.753691][    T8] x14: 0000000000000000 x13: 0000000000000f58
> > > x12: 0000000082355555
> > > [   10.753715][    T8] x11: 00006acfbfa2f400 x10: 0000000000000016 x9
> > > : 00ffffffffffffff
> > > [   10.753740][    T8] x8 : da9ecda1b63b0500 x7 : 0000000000008080 x6
> > > : 0000000000000000
> > > [   10.753764][    T8] x5 : 0000000000000001 x4 : 0000646e756f626e x3
> > > : ffffff801ad99828
> > > [   10.753788][    T8] x2 : ffffff8003385da8 x1 : ffffffc009ed3d20 x0
> > > : ffffff8003143d20
> > > [   10.753812][    T8] Call trace:
> > > [   10.753818][    T8]  regulator_bulk_enable_async+0x3c/0x98
> > > [   10.753839][    T8]  async_run_entry_fn+0x30/0xf8
> > > [   10.753859][    T8]  process_one_work+0x1d0/0x404
> > > [   10.753879][    T8]  worker_thread+0x25c/0x43c
> > > [   10.753897][    T8]  kthread+0xf0/0x2c0
> > > [   10.753912][    T8]  ret_from_fork+0x10/0x20
> > > [   10.753940][    T8] Code: f81f83a8 f9400814 a900ffff f90003ff (f9403e95)
> > > [   10.753950][    T8] ---[ end trace 0000000000000000 ]---
> > > [   10.760621][  T194] apexd: Found pre-installed APEX
> > > /system/apex/com.android.permission.capex
> > > [   10.767672][   T67] Mem abort info:
> > > [   10.779658][  T194] apexd: Found pre-installed APEX
> > > /system/apex/com.android.art.capex
> > > [   10.783690][   T67]   ESR = 0x0000000096000005
> > > [   10.792424][  T194] apexd: Found pre-installed APEX
> > > /system/apex/com.android.scheduling.capex
> > > [   10.794713][    T8] Kernel panic - not syncing: Oops: Fatal exception
> > > [   10.794723][    T8] SMP: stopping secondary CPUs
> > > [   10.798141][    T8] Kernel Offset: 0x70000 from 0xffffffc008000000
> > > [   10.798150][    T8] PHYS_OFFSET: 0x0
> > > [   10.798156][    T8] CPU features: 0x0000,00649020,00001086
> > > [   10.798167][    T8] Memory Limit: none
> >
> > Are you fixed by the patch ("regulator: core: Fix missing error return
> > from regulator_bulk_get()") [1]
> >
> > [1] https://lore.kernel.org/r/20220809142738.1.I91625242f137c707bb345c51c80c5ecee02eeff3@changeid
>
> Thanks for the check!
> I tested the above patch with the android-mainline based kernel,
> and it fixes the regulator_bulk_enable_async kernel panic reported here.
>
> --
> Best Regards,
> Yongqin Liu
> ---------------------------------------------------------------
> #mailing list
> linaro-android@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-android



-- 
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API
  2022-08-23  6:22         ` Yongqin Liu
@ 2022-08-23 14:50           ` Doug Anderson
  2022-08-23 18:24             ` Yongqin Liu
  0 siblings, 1 reply; 16+ messages in thread
From: Doug Anderson @ 2022-08-23 14:50 UTC (permalink / raw)
  To: Yongqin Liu
  Cc: linux-arm-msm, Abhinav Kumar, dri-devel, Liam Girdwood,
	Steve Muckle, Alistair Delva, Mark Brown, John Stultz,
	Dmitry Baryshkov, freedreno, Sumit Semwal, LKML, Todd Kjos

Hi,

On Mon, Aug 22, 2022 at 11:23 PM Yongqin Liu <yongqin.liu@linaro.org> wrote:
>
> Hi, Douglas
>
> Just an update on the fix you pointed out previously here:
> > > [1] https://lore.kernel.org/r/20220809142738.1.I91625242f137c707bb345c51c80c5ecee02eeff3@changeid
>
> With it I could boot the hikey960 build to the home screen if it does
> not use the GKI kernel.
> but the problem will be reproduced if it uses the GKI kernel.
>
> And if this change is reverted, then it could boot with the GKI kernel as well.
>
> I am not sure what's the reason there, but there seems to be some
> difference with the fix above and the workaround of revert.
> Not sure if you have any idea about that.
>
> Regarding the GKI kernel(Android Generic Kernel Image)[2], it's built
> from the android-mainline tree(f51334eac4de) without any workaround.
> (Neither the revert, nor the fix applied), and the regulator modules
> used for the hikey960 build are hi6421v530-regulator.ko and
> hi655x-regulator.ko
>
> I am still not sure if it would work with the GKI kernel that has the
> fix that you pointed out in. the case that both the GKI kernel and
> vendor tree have the fix.
> Will update here when I have some results.
>
> [2]: https://source.android.com/docs/core/architecture/kernel/generic-kernel-image?hl=en

That's not too surprising. The broken patch is in the core kernel so
you need the fix in the core kernel. I think that means you'll have to
wait until `android-mainline` gets the fix. I don't work on Android,
so if there's some other route to get an expedited fix into
android-mainline I'm not aware of it.

-Doug

-Doug

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API
  2022-08-23 14:50           ` Doug Anderson
@ 2022-08-23 18:24             ` Yongqin Liu
  0 siblings, 0 replies; 16+ messages in thread
From: Yongqin Liu @ 2022-08-23 18:24 UTC (permalink / raw)
  To: Doug Anderson
  Cc: linux-arm-msm, Abhinav Kumar, dri-devel, Liam Girdwood,
	Steve Muckle, Alistair Delva, Mark Brown, John Stultz,
	Dmitry Baryshkov, freedreno, Sumit Semwal, LKML, Todd Kjos

Hi, Douglas

On Tue, 23 Aug 2022 at 22:50, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Aug 22, 2022 at 11:23 PM Yongqin Liu <yongqin.liu@linaro.org> wrote:
> >
> > Hi, Douglas
> >
> > Just an update on the fix you pointed out previously here:
> > > > [1] https://lore.kernel.org/r/20220809142738.1.I91625242f137c707bb345c51c80c5ecee02eeff3@changeid
> >
> > With it I could boot the hikey960 build to the home screen if it does
> > not use the GKI kernel.
> > but the problem will be reproduced if it uses the GKI kernel.
> >
> > And if this change is reverted, then it could boot with the GKI kernel as well.
> >
> > I am not sure what's the reason there, but there seems to be some
> > difference with the fix above and the workaround of revert.
> > Not sure if you have any idea about that.
> >
> > Regarding the GKI kernel(Android Generic Kernel Image)[2], it's built
> > from the android-mainline tree(f51334eac4de) without any workaround.
> > (Neither the revert, nor the fix applied), and the regulator modules
> > used for the hikey960 build are hi6421v530-regulator.ko and
> > hi655x-regulator.ko
> >
> > I am still not sure if it would work with the GKI kernel that has the
> > fix that you pointed out in. the case that both the GKI kernel and
> > vendor tree have the fix.
> > Will update here when I have some results.

Just checked, with the fix applied in the GKI kernel, the problem is
not reproduced again.

> > [2]: https://source.android.com/docs/core/architecture/kernel/generic-kernel-image?hl=en
>
> That's not too surprising. The broken patch is in the core kernel so
> you need the fix in the core kernel.
Sorry, I still do not get the point here.

The GKI kernel is the same one, that does not have the revert and the
fix applied.

for the vendor tree(the ko files and dtb files are used)
#1 built with this commit reverted.
#2 built with the fix applied.

#1 could boot with the GKI kernel, while #2 does not boot with the same error.
What might cause the difference?

> I think that means you'll have to
> wait until `android-mainline` gets the fix. I don't work on Android,
> so if there's some other route to get an expedited fix into
> android-mainline I'm not aware of it.

Thanks, I will wait for the fix to be merged into the android-mainline,
before that I will use the revert workaround for the moment.

-- 
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-08-23 18:24 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-26 17:38 [PATCH v2 0/7] drm/msm/dsi regulator improvements Douglas Anderson
2022-07-26 17:38 ` [PATCH v2 1/7] drm/msm/dsi: Fix number of regulators for msm8996_dsi_cfg Douglas Anderson
2022-07-26 17:38 ` [PATCH v2 2/7] drm/msm/dsi: Fix number of regulators for SDM660 Douglas Anderson
2022-07-26 17:38 ` [PATCH v2 3/7] drm/msm/dsi: Don't set a load before disabling a regulator Douglas Anderson
2022-07-26 17:38 ` [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API Douglas Anderson
2022-08-16 12:58   ` Yongqin Liu
2022-08-17 14:25     ` Doug Anderson
2022-08-17 16:52       ` Yongqin Liu
2022-08-23  6:22         ` Yongqin Liu
2022-08-23 14:50           ` Doug Anderson
2022-08-23 18:24             ` Yongqin Liu
2022-07-26 17:38 ` [PATCH v2 5/7] drm/msm/dsi: Use the new regulator bulk feature to specify the load Douglas Anderson
2022-07-26 17:38 ` [PATCH v2 6/7] regulator: core: Allow drivers to define their init data as const Douglas Anderson
2022-07-26 17:38 ` [PATCH v2 7/7] drm/msm/dsi: Take advantage of devm_regulator_bulk_get_const() Douglas Anderson
2022-07-27 23:02 ` (subset) [PATCH v2 0/7] drm/msm/dsi regulator improvements Mark Brown
2022-07-28  0:20 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).