* [PATCH 0/6] Use correct LDO5 control registers for PCA9450
@ 2023-02-13 15:58 ` Frieder Schrempf
0 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 15:58 UTC (permalink / raw)
To: devicetree, Frieder Schrempf, linux-arm-kernel, linux-kernel,
Mark Brown, Robin Gong
Cc: Marek Vasut, Krzysztof Kozlowski, Liam Girdwood, Rob Herring,
Sascha Hauer, Shawn Guo
From: Frieder Schrempf <frieder.schrempf@kontron.de>
This patchset fixes the control of the LDO5 regulator by providing an
option for letting the driver know which of the two possible control
registers is currently in use by the hardware.
It also fixes the enable register for LDO5 to use PCA9450_REG_LDO5CTRL_L
as specified by the datasheet.
The last patch makes use of the fix by adjusting the devicetree for
the Kontron i.MX8MM boards.
In Linux this currently doesn't fix any functional issues, but in
U-Boot similar changes are needed in order to fix SD card access.
See the following thread for more information:
https://lists.denx.de/pipermail/u-boot/2023-January/506103.html
Frieder Schrempf (6):
dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios
regulator: pca9450: Fix enable register for LDO5
Revert "regulator: pca9450: Add SD_VSEL GPIO for LDO5"
regulator: Add operation to let drivers select vsel register
regulator: pca9450: Fix control register for LDO5
arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
.../regulator/nxp,pca9450-regulator.yaml | 23 ++++++---
.../dts/freescale/imx8mm-kontron-bl-osm-s.dts | 6 +--
.../boot/dts/freescale/imx8mm-kontron-bl.dts | 6 +--
.../dts/freescale/imx8mm-kontron-osm-s.dtsi | 1 +
.../boot/dts/freescale/imx8mm-kontron-sl.dtsi | 1 +
drivers/regulator/helpers.c | 16 ++++++-
drivers/regulator/pca9450-regulator.c | 47 ++++++++++++++-----
include/linux/regulator/driver.h | 5 ++
8 files changed, 79 insertions(+), 26 deletions(-)
--
2.39.1
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 0/6] Use correct LDO5 control registers for PCA9450
@ 2023-02-13 15:58 ` Frieder Schrempf
0 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 15:58 UTC (permalink / raw)
To: devicetree, Frieder Schrempf, linux-arm-kernel, linux-kernel,
Mark Brown, Robin Gong
Cc: Marek Vasut, Krzysztof Kozlowski, Liam Girdwood, Rob Herring,
Sascha Hauer, Shawn Guo
From: Frieder Schrempf <frieder.schrempf@kontron.de>
This patchset fixes the control of the LDO5 regulator by providing an
option for letting the driver know which of the two possible control
registers is currently in use by the hardware.
It also fixes the enable register for LDO5 to use PCA9450_REG_LDO5CTRL_L
as specified by the datasheet.
The last patch makes use of the fix by adjusting the devicetree for
the Kontron i.MX8MM boards.
In Linux this currently doesn't fix any functional issues, but in
U-Boot similar changes are needed in order to fix SD card access.
See the following thread for more information:
https://lists.denx.de/pipermail/u-boot/2023-January/506103.html
Frieder Schrempf (6):
dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios
regulator: pca9450: Fix enable register for LDO5
Revert "regulator: pca9450: Add SD_VSEL GPIO for LDO5"
regulator: Add operation to let drivers select vsel register
regulator: pca9450: Fix control register for LDO5
arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
.../regulator/nxp,pca9450-regulator.yaml | 23 ++++++---
.../dts/freescale/imx8mm-kontron-bl-osm-s.dts | 6 +--
.../boot/dts/freescale/imx8mm-kontron-bl.dts | 6 +--
.../dts/freescale/imx8mm-kontron-osm-s.dtsi | 1 +
.../boot/dts/freescale/imx8mm-kontron-sl.dtsi | 1 +
drivers/regulator/helpers.c | 16 ++++++-
drivers/regulator/pca9450-regulator.c | 47 ++++++++++++++-----
include/linux/regulator/driver.h | 5 ++
8 files changed, 79 insertions(+), 26 deletions(-)
--
2.39.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 1/6] dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios
2023-02-13 15:58 ` Frieder Schrempf
(?)
@ 2023-02-13 15:58 ` Frieder Schrempf
2023-02-15 20:02 ` Rob Herring
-1 siblings, 1 reply; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 15:58 UTC (permalink / raw)
To: devicetree, Krzysztof Kozlowski, Liam Girdwood, linux-kernel,
Mark Brown, Rob Herring, Robin Gong
Cc: Marek Vasut, Frieder Schrempf, Per-Daniel Olsson, Rickard x Andersson
From: Frieder Schrempf <frieder.schrempf@kontron.de>
The sd-vsel-gpios property is abandoned in its current meaning as an
output. We now use it to specify an optional signal that can be
evaluated by the driver in order to retrieve the current status
of the SD_VSEL signal that is used to select the control register
of LDO5.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
.../regulator/nxp,pca9450-regulator.yaml | 23 ++++++++++++++-----
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
index 835b53302db8..c86534538a4e 100644
--- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
@@ -40,8 +40,24 @@ properties:
description: |
list of regulators provided by this controller
+ properties:
+ LDO5:
+ type: object
+ $ref: regulator.yaml#
+ description:
+ Properties for single LDO5 regulator.
+
+ properties:
+ sd-vsel-gpios:
+ description:
+ GPIO that can be used to read the current status of the SD_VSEL
+ signal in order for the driver to know if LDO5CTRL_L or LDO5CTRL_H
+ is used by the hardware.
+
+ unevaluatedProperties: false
+
patternProperties:
- "^LDO[1-5]$":
+ "^LDO[1-4]$":
type: object
$ref: regulator.yaml#
description:
@@ -76,11 +92,6 @@ properties:
additionalProperties: false
- sd-vsel-gpios:
- description: GPIO that is used to switch LDO5 between being configured by
- LDO5CTRL_L or LDO5CTRL_H register. Use this if the SD_VSEL signal is
- connected to a host GPIO.
-
nxp,i2c-lt-enable:
type: boolean
description:
--
2.39.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 2/6] regulator: pca9450: Fix enable register for LDO5
2023-02-13 15:58 ` Frieder Schrempf
(?)
(?)
@ 2023-02-13 15:58 ` Frieder Schrempf
2023-02-13 16:12 ` Marek Vasut
-1 siblings, 1 reply; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 15:58 UTC (permalink / raw)
To: Frieder Schrempf, Liam Girdwood, linux-kernel, Mark Brown, Robin Gong
Cc: Marek Vasut, Per-Daniel Olsson, Rickard x Andersson,
Uwe Kleine-König
From: Frieder Schrempf <frieder.schrempf@kontron.de>
The LDO5 regulator has two configuration registers, but only
LDO5CTRL_L contains the bits for enabling/disabling the regulator.
Fixes: 0935ff5f1f0a ("regulator: pca9450: add pca9450 pmic driver")
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
drivers/regulator/pca9450-regulator.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
index c6351fac9f4d..a815666566b5 100644
--- a/drivers/regulator/pca9450-regulator.c
+++ b/drivers/regulator/pca9450-regulator.c
@@ -447,7 +447,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
.n_linear_ranges = ARRAY_SIZE(pca9450_ldo5_volts),
.vsel_reg = PCA9450_REG_LDO5CTRL_H,
.vsel_mask = LDO5HOUT_MASK,
- .enable_reg = PCA9450_REG_LDO5CTRL_H,
+ .enable_reg = PCA9450_REG_LDO5CTRL_L,
.enable_mask = LDO5H_EN_MASK,
.owner = THIS_MODULE,
},
@@ -656,7 +656,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
.n_linear_ranges = ARRAY_SIZE(pca9450_ldo5_volts),
.vsel_reg = PCA9450_REG_LDO5CTRL_H,
.vsel_mask = LDO5HOUT_MASK,
- .enable_reg = PCA9450_REG_LDO5CTRL_H,
+ .enable_reg = PCA9450_REG_LDO5CTRL_L,
.enable_mask = LDO5H_EN_MASK,
.owner = THIS_MODULE,
},
--
2.39.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 3/6] Revert "regulator: pca9450: Add SD_VSEL GPIO for LDO5"
2023-02-13 15:58 ` Frieder Schrempf
` (2 preceding siblings ...)
(?)
@ 2023-02-13 15:58 ` Frieder Schrempf
2023-02-13 16:16 ` Marek Vasut
-1 siblings, 1 reply; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 15:58 UTC (permalink / raw)
To: Liam Girdwood, linux-kernel, Mark Brown
Cc: Marek Vasut, Frieder Schrempf, Per-Daniel Olsson,
Rickard x Andersson, Uwe Kleine-König
From: Frieder Schrempf <frieder.schrempf@kontron.de>
This reverts commit 8c67a11bae889f51fe5054364c3c789dfae3ad73.
It turns out that all boards using the PCA9450 actually have the
SD_VSEL input conencted to the VSELECT signal of the SoCs SD/MMC
interface. Therefore we don't need manual control for this signal
via GPIO and threre aren't any users.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
drivers/regulator/pca9450-regulator.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
index a815666566b5..804a22c0e376 100644
--- a/drivers/regulator/pca9450-regulator.c
+++ b/drivers/regulator/pca9450-regulator.c
@@ -5,7 +5,6 @@
*/
#include <linux/err.h>
-#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
@@ -33,7 +32,6 @@ struct pca9450_regulator_desc {
struct pca9450 {
struct device *dev;
struct regmap *regmap;
- struct gpio_desc *sd_vsel_gpio;
enum pca9450_chip_type type;
unsigned int rcnt;
int irq;
@@ -834,18 +832,6 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
}
}
- /*
- * The driver uses the LDO5CTRL_H register to control the LDO5 regulator.
- * This is only valid if the SD_VSEL input of the PMIC is high. Let's
- * check if the pin is available as GPIO and set it to high.
- */
- pca9450->sd_vsel_gpio = gpiod_get_optional(pca9450->dev, "sd-vsel", GPIOD_OUT_HIGH);
-
- if (IS_ERR(pca9450->sd_vsel_gpio)) {
- dev_err(&i2c->dev, "Failed to get SD_VSEL GPIO\n");
- return PTR_ERR(pca9450->sd_vsel_gpio);
- }
-
dev_info(&i2c->dev, "%s probed.\n",
type == PCA9450_TYPE_PCA9450A ? "pca9450a" : "pca9450bc");
--
2.39.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 4/6] regulator: Add operation to let drivers select vsel register
2023-02-13 15:58 ` Frieder Schrempf
` (3 preceding siblings ...)
(?)
@ 2023-02-13 15:58 ` Frieder Schrempf
2023-02-14 1:40 ` kernel test robot
` (2 more replies)
-1 siblings, 3 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 15:58 UTC (permalink / raw)
To: Liam Girdwood, linux-kernel, Mark Brown
Cc: Marek Vasut, Frieder Schrempf, ChiYuan Huang, Mauro Carvalho Chehab
From: Frieder Schrempf <frieder.schrempf@kontron.de>
There are regulators that use multiple registers for storing the
voltage. Add a get_reg_voltage_sel member to struct regulator_ops in
order to let drivers register a function that returns the currently
used register.
The pca9450 driver will be a user of this as the LDO5 regulator of
that chip uses two different control registers depending on the
state of an external signal.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
drivers/regulator/helpers.c | 16 ++++++++++++++--
include/linux/regulator/driver.h | 5 +++++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index ad2237a95572..e629b0bea3d0 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -223,6 +223,16 @@ int regulator_set_voltage_sel_pickable_regmap(struct regulator_dev *rdev,
}
EXPORT_SYMBOL_GPL(regulator_set_voltage_sel_pickable_regmap);
+unsigned int regulator_get_hwreg_voltage_sel_regmap(struct regulator_dev *rdev)
+{
+ const struct regulator_ops *ops = rdev->desc->ops;
+
+ if (ops->get_reg_voltage_sel)
+ return ops->get_reg_voltage_sel(rdev);
+
+ return rdev->desc->vsel_reg;
+}
+
/**
* regulator_get_voltage_sel_regmap - standard get_voltage_sel for regmap users
*
@@ -234,10 +244,11 @@ EXPORT_SYMBOL_GPL(regulator_set_voltage_sel_pickable_regmap);
*/
int regulator_get_voltage_sel_regmap(struct regulator_dev *rdev)
{
+ unsigned int vsel_reg = regulator_get_hwreg_voltage_sel_regmap(rdev);
unsigned int val;
int ret;
- ret = regmap_read(rdev->regmap, rdev->desc->vsel_reg, &val);
+ ret = regmap_read(rdev->regmap, vsel_reg, &val);
if (ret != 0)
return ret;
@@ -260,11 +271,12 @@ EXPORT_SYMBOL_GPL(regulator_get_voltage_sel_regmap);
*/
int regulator_set_voltage_sel_regmap(struct regulator_dev *rdev, unsigned sel)
{
+ unsigned int vsel_reg = regulator_get_hwreg_voltage_sel_regmap(rdev);
int ret;
sel <<= ffs(rdev->desc->vsel_mask) - 1;
- ret = regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg,
+ ret = regmap_update_bits(rdev->regmap, vsel_reg,
rdev->desc->vsel_mask, sel);
if (ret)
return ret;
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index d3b4a3d4514a..c9953b2f63d5 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -77,6 +77,10 @@ enum regulator_detection_severity {
* @get_voltage_sel: Return the currently configured voltage selector for the
* regulator; return -ENOTRECOVERABLE if regulator can't
* be read at bootup and hasn't been set yet.
+ * @get_reg_voltage_sel: Return the register used for getting/setting the
+ * voltage of the regulator. This is useful if the
+ * regulator uses multiple registers internally, switched
+ * by some condition like the state of an external signal.
* @list_voltage: Return one of the supported voltages, in microvolts; zero
* if the selector indicates a voltage that is unusable on this system;
* or negative errno. Selectors range from zero to one less than
@@ -168,6 +172,7 @@ struct regulator_ops {
int (*set_voltage_sel) (struct regulator_dev *, unsigned selector);
int (*get_voltage) (struct regulator_dev *);
int (*get_voltage_sel) (struct regulator_dev *);
+ unsigned int (*get_reg_voltage_sel) (struct regulator_dev *);
/* get/set regulator current */
int (*set_current_limit) (struct regulator_dev *,
--
2.39.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 5/6] regulator: pca9450: Fix control register for LDO5
2023-02-13 15:58 ` Frieder Schrempf
` (4 preceding siblings ...)
(?)
@ 2023-02-13 15:58 ` Frieder Schrempf
-1 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 15:58 UTC (permalink / raw)
To: Liam Girdwood, linux-kernel, Mark Brown
Cc: Marek Vasut, Frieder Schrempf, Per-Daniel Olsson,
Rickard x Andersson, Uwe Kleine-König
From: Frieder Schrempf <frieder.schrempf@kontron.de>
For LDO5 we need to be able to check the status of the SD_VSEL input in
order to know which control register is used. Read the status of the
SD_VSEL signal via GPIO and add a get_reg_voltage_sel operation for
LDO5 that returns the register that is currently in use to the core.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
drivers/regulator/pca9450-regulator.c | 45 ++++++++++++++++++++++++---
1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
index 804a22c0e376..a0802c6cb259 100644
--- a/drivers/regulator/pca9450-regulator.c
+++ b/drivers/regulator/pca9450-regulator.c
@@ -5,6 +5,7 @@
*/
#include <linux/err.h>
+#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
@@ -32,6 +33,7 @@ struct pca9450_regulator_desc {
struct pca9450 {
struct device *dev;
struct regmap *regmap;
+ struct gpio_desc *sd_vsel_gpio;
enum pca9450_chip_type type;
unsigned int rcnt;
int irq;
@@ -55,6 +57,16 @@ static const struct regmap_config pca9450_regmap_config = {
.cache_type = REGCACHE_RBTREE,
};
+static unsigned int pca9450_ldo5_get_reg_voltage_sel(struct regulator_dev *rdev)
+{
+ struct pca9450 *pca9450 = rdev_get_drvdata(rdev);
+
+ if (pca9450->sd_vsel_gpio && !gpiod_get_value(pca9450->sd_vsel_gpio))
+ return PCA9450_REG_LDO5CTRL_L;
+
+ return PCA9450_REG_LDO5CTRL_H;
+}
+
/*
* BUCK1/2/3
* BUCK1RAM[1:0] BUCK1 DVS ramp rate setting
@@ -97,6 +109,16 @@ static const struct regulator_ops pca9450_ldo_regulator_ops = {
.get_voltage_sel = regulator_get_voltage_sel_regmap,
};
+static const struct regulator_ops pca9450_ldo5_regulator_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear_range,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .get_reg_voltage_sel = pca9450_ldo5_get_reg_voltage_sel,
+};
+
/*
* BUCK1/2/3
* 0.60 to 2.1875V (12.5mV step)
@@ -438,12 +460,11 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
.of_match = of_match_ptr("LDO5"),
.regulators_node = of_match_ptr("regulators"),
.id = PCA9450_LDO5,
- .ops = &pca9450_ldo_regulator_ops,
+ .ops = &pca9450_ldo5_regulator_ops,
.type = REGULATOR_VOLTAGE,
.n_voltages = PCA9450_LDO5_VOLTAGE_NUM,
.linear_ranges = pca9450_ldo5_volts,
.n_linear_ranges = ARRAY_SIZE(pca9450_ldo5_volts),
- .vsel_reg = PCA9450_REG_LDO5CTRL_H,
.vsel_mask = LDO5HOUT_MASK,
.enable_reg = PCA9450_REG_LDO5CTRL_L,
.enable_mask = LDO5H_EN_MASK,
@@ -647,12 +668,11 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
.of_match = of_match_ptr("LDO5"),
.regulators_node = of_match_ptr("regulators"),
.id = PCA9450_LDO5,
- .ops = &pca9450_ldo_regulator_ops,
+ .ops = &pca9450_ldo5_regulator_ops,
.type = REGULATOR_VOLTAGE,
.n_voltages = PCA9450_LDO5_VOLTAGE_NUM,
.linear_ranges = pca9450_ldo5_volts,
.n_linear_ranges = ARRAY_SIZE(pca9450_ldo5_volts),
- .vsel_reg = PCA9450_REG_LDO5CTRL_H,
.vsel_mask = LDO5HOUT_MASK,
.enable_reg = PCA9450_REG_LDO5CTRL_L,
.enable_mask = LDO5H_EN_MASK,
@@ -705,6 +725,7 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
of_device_get_match_data(&i2c->dev);
const struct pca9450_regulator_desc *regulator_desc;
struct regulator_config config = { };
+ struct regulator_dev *ldo5;
struct pca9450 *pca9450;
unsigned int device_id, i;
unsigned int reset_ctrl;
@@ -770,6 +791,7 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
config.regmap = pca9450->regmap;
config.dev = pca9450->dev;
+ config.driver_data = pca9450;
rdev = devm_regulator_register(pca9450->dev, desc, &config);
if (IS_ERR(rdev)) {
@@ -779,6 +801,9 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
desc->name, ret);
return ret;
}
+
+ if (!strcmp(desc->name, "ldo5"))
+ ldo5 = rdev;
}
ret = devm_request_threaded_irq(pca9450->dev, pca9450->irq, NULL,
@@ -832,6 +857,18 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
}
}
+ /*
+ * For LDO5 we need to be able to check the status of the SD_VSEL input in
+ * order to know which control register is used. Most boards connect SD_VSEL
+ * to the VSELECT signal, so we can use the GPIO that is internally routed
+ * to this signal (if SION bit is set in IOMUX).
+ */
+ pca9450->sd_vsel_gpio = gpiod_get_optional(&ldo5->dev, "sd-vsel", GPIOD_IN);
+ if (IS_ERR(pca9450->sd_vsel_gpio)) {
+ dev_err(&i2c->dev, "Failed to get SD_VSEL GPIO\n");
+ return ret;
+ }
+
dev_info(&i2c->dev, "%s probed.\n",
type == PCA9450_TYPE_PCA9450A ? "pca9450a" : "pca9450bc");
--
2.39.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 15:58 ` Frieder Schrempf
@ 2023-02-13 15:58 ` Frieder Schrempf
-1 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 15:58 UTC (permalink / raw)
To: devicetree, Krzysztof Kozlowski, linux-arm-kernel, linux-kernel,
Rob Herring, Sascha Hauer, Shawn Guo
Cc: Marek Vasut, Frieder Schrempf, Fabio Estevam, Heiko Thiery,
Krzysztof Kozlowski, NXP Linux Team, Oleksij Rempel,
Pengutronix Kernel Team
From: Frieder Schrempf <frieder.schrempf@kontron.de>
This fixes the LDO5 regulator handling of the pca9450 driver by
taking the status of the SD_VSEL into account to determine which
configuration register is used for the voltage setting.
Even without this change there is no functional issue, as the code
for switching the voltage in sdhci.c currently switches both, the
VSELECT/SD_VSEL signal and the regulator voltage at the same time
and doesn't run into an invalid corner case.
We should still make sure, that we always use the correct register
when controlling the regulator. At least in U-Boot this fixes an
actual bug where the wrong IO voltage is used.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts | 6 +++---
arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts | 6 +++---
arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi | 1 +
arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi | 1 +
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
index 8b16bd68576c..bdcd9cd843c7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
@@ -344,7 +344,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
@@ -357,7 +357,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
@@ -370,7 +370,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
index a079322a3793..ba2a4491cf31 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
@@ -321,7 +321,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
@@ -334,7 +334,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
@@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
index 5172883717d1..90daaf54e704 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
@@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
regulator-name = "NVCC_SD (LDO5)";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <3300000>;
+ sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
};
};
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
index 1f8326613ee9..7468a8aa771d 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
@@ -195,6 +195,7 @@ reg_nvcc_sd: LDO5 {
regulator-name = "NVCC_SD (LDO5)";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <3300000>;
+ sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
};
};
};
--
2.39.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-13 15:58 ` Frieder Schrempf
0 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 15:58 UTC (permalink / raw)
To: devicetree, Krzysztof Kozlowski, linux-arm-kernel, linux-kernel,
Rob Herring, Sascha Hauer, Shawn Guo
Cc: Marek Vasut, Frieder Schrempf, Fabio Estevam, Heiko Thiery,
Krzysztof Kozlowski, NXP Linux Team, Oleksij Rempel,
Pengutronix Kernel Team
From: Frieder Schrempf <frieder.schrempf@kontron.de>
This fixes the LDO5 regulator handling of the pca9450 driver by
taking the status of the SD_VSEL into account to determine which
configuration register is used for the voltage setting.
Even without this change there is no functional issue, as the code
for switching the voltage in sdhci.c currently switches both, the
VSELECT/SD_VSEL signal and the regulator voltage at the same time
and doesn't run into an invalid corner case.
We should still make sure, that we always use the correct register
when controlling the regulator. At least in U-Boot this fixes an
actual bug where the wrong IO voltage is used.
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts | 6 +++---
arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts | 6 +++---
arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi | 1 +
arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi | 1 +
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
index 8b16bd68576c..bdcd9cd843c7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
@@ -344,7 +344,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
@@ -357,7 +357,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
@@ -370,7 +370,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
index a079322a3793..ba2a4491cf31 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
@@ -321,7 +321,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
@@ -334,7 +334,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
@@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
- MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
+ MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>;
};
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
index 5172883717d1..90daaf54e704 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
@@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
regulator-name = "NVCC_SD (LDO5)";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <3300000>;
+ sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
};
};
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
index 1f8326613ee9..7468a8aa771d 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
@@ -195,6 +195,7 @@ reg_nvcc_sd: LDO5 {
regulator-name = "NVCC_SD (LDO5)";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <3300000>;
+ sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
};
};
};
--
2.39.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 15:58 ` Frieder Schrempf
@ 2023-02-13 16:08 ` Marek Vasut
-1 siblings, 0 replies; 45+ messages in thread
From: Marek Vasut @ 2023-02-13 16:08 UTC (permalink / raw)
To: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo
Cc: Frieder Schrempf, Fabio Estevam, Heiko Thiery,
Krzysztof Kozlowski, NXP Linux Team, Oleksij Rempel,
Pengutronix Kernel Team
On 2/13/23 16:58, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> This fixes the LDO5 regulator handling of the pca9450 driver by
> taking the status of the SD_VSEL into account to determine which
> configuration register is used for the voltage setting.
>
> Even without this change there is no functional issue, as the code
> for switching the voltage in sdhci.c currently switches both, the
> VSELECT/SD_VSEL signal and the regulator voltage at the same time
> and doesn't run into an invalid corner case.
>
> We should still make sure, that we always use the correct register
> when controlling the regulator. At least in U-Boot this fixes an
> actual bug where the wrong IO voltage is used.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
I may have a kind-of obvious request, since we removed those SD_VSEL
very recently from other boards, could you just revert all those patches
and only fill in the SION bit in V2 on all those boards too ? That way,
we fix the LDO5 regulator for everyone who used it before.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-13 16:08 ` Marek Vasut
0 siblings, 0 replies; 45+ messages in thread
From: Marek Vasut @ 2023-02-13 16:08 UTC (permalink / raw)
To: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo
Cc: Frieder Schrempf, Fabio Estevam, Heiko Thiery,
Krzysztof Kozlowski, NXP Linux Team, Oleksij Rempel,
Pengutronix Kernel Team
On 2/13/23 16:58, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> This fixes the LDO5 regulator handling of the pca9450 driver by
> taking the status of the SD_VSEL into account to determine which
> configuration register is used for the voltage setting.
>
> Even without this change there is no functional issue, as the code
> for switching the voltage in sdhci.c currently switches both, the
> VSELECT/SD_VSEL signal and the regulator voltage at the same time
> and doesn't run into an invalid corner case.
>
> We should still make sure, that we always use the correct register
> when controlling the regulator. At least in U-Boot this fixes an
> actual bug where the wrong IO voltage is used.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
I may have a kind-of obvious request, since we removed those SD_VSEL
very recently from other boards, could you just revert all those patches
and only fill in the SION bit in V2 on all those boards too ? That way,
we fix the LDO5 regulator for everyone who used it before.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 2/6] regulator: pca9450: Fix enable register for LDO5
2023-02-13 15:58 ` [PATCH 2/6] regulator: pca9450: Fix enable register for LDO5 Frieder Schrempf
@ 2023-02-13 16:12 ` Marek Vasut
0 siblings, 0 replies; 45+ messages in thread
From: Marek Vasut @ 2023-02-13 16:12 UTC (permalink / raw)
To: Frieder Schrempf, Frieder Schrempf, Liam Girdwood, linux-kernel,
Mark Brown, Robin Gong
Cc: Per-Daniel Olsson, Rickard x Andersson, Uwe Kleine-König
On 2/13/23 16:58, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> The LDO5 regulator has two configuration registers, but only
> LDO5CTRL_L contains the bits for enabling/disabling the regulator.
>
> Fixes: 0935ff5f1f0a ("regulator: pca9450: add pca9450 pmic driver")
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Marek Vasut <marex@denx.de>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 16:08 ` Marek Vasut
@ 2023-02-13 16:15 ` Frieder Schrempf
-1 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 16:15 UTC (permalink / raw)
To: Marek Vasut, Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo
Cc: Fabio Estevam, Heiko Thiery, Krzysztof Kozlowski, NXP Linux Team,
Oleksij Rempel, Pengutronix Kernel Team
On 13.02.23 17:08, Marek Vasut wrote:
> On 2/13/23 16:58, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> This fixes the LDO5 regulator handling of the pca9450 driver by
>> taking the status of the SD_VSEL into account to determine which
>> configuration register is used for the voltage setting.
>>
>> Even without this change there is no functional issue, as the code
>> for switching the voltage in sdhci.c currently switches both, the
>> VSELECT/SD_VSEL signal and the regulator voltage at the same time
>> and doesn't run into an invalid corner case.
>>
>> We should still make sure, that we always use the correct register
>> when controlling the regulator. At least in U-Boot this fixes an
>> actual bug where the wrong IO voltage is used.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> I may have a kind-of obvious request, since we removed those SD_VSEL
> very recently from other boards, could you just revert all those patches
> and only fill in the SION bit in V2 on all those boards too ? That way,
> we fix the LDO5 regulator for everyone who used it before.
I thought about that, but as the SD_VSEL signal is controlling the LDO5,
I found it more useful/correct to have the sd-vsel-gpios property inside
the LDO5 regulator node. Therefore the old usage where sd-vsel-gpios was
inside the PMIC node isn't compatible anymore.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-13 16:15 ` Frieder Schrempf
0 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 16:15 UTC (permalink / raw)
To: Marek Vasut, Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo
Cc: Fabio Estevam, Heiko Thiery, Krzysztof Kozlowski, NXP Linux Team,
Oleksij Rempel, Pengutronix Kernel Team
On 13.02.23 17:08, Marek Vasut wrote:
> On 2/13/23 16:58, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> This fixes the LDO5 regulator handling of the pca9450 driver by
>> taking the status of the SD_VSEL into account to determine which
>> configuration register is used for the voltage setting.
>>
>> Even without this change there is no functional issue, as the code
>> for switching the voltage in sdhci.c currently switches both, the
>> VSELECT/SD_VSEL signal and the regulator voltage at the same time
>> and doesn't run into an invalid corner case.
>>
>> We should still make sure, that we always use the correct register
>> when controlling the regulator. At least in U-Boot this fixes an
>> actual bug where the wrong IO voltage is used.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> I may have a kind-of obvious request, since we removed those SD_VSEL
> very recently from other boards, could you just revert all those patches
> and only fill in the SION bit in V2 on all those boards too ? That way,
> we fix the LDO5 regulator for everyone who used it before.
I thought about that, but as the SD_VSEL signal is controlling the LDO5,
I found it more useful/correct to have the sd-vsel-gpios property inside
the LDO5 regulator node. Therefore the old usage where sd-vsel-gpios was
inside the PMIC node isn't compatible anymore.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 15:58 ` Frieder Schrempf
@ 2023-02-13 16:15 ` Marco Felsch
-1 siblings, 0 replies; 45+ messages in thread
From: Marco Felsch @ 2023-02-13 16:15 UTC (permalink / raw)
To: Frieder Schrempf
Cc: devicetree, Krzysztof Kozlowski, linux-arm-kernel, linux-kernel,
Rob Herring, Sascha Hauer, Shawn Guo, Marek Vasut,
Frieder Schrempf, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery,
Fabio Estevam
Hi Frieder,
thanks for the patch.
On 23-02-13, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> This fixes the LDO5 regulator handling of the pca9450 driver by
> taking the status of the SD_VSEL into account to determine which
> configuration register is used for the voltage setting.
>
> Even without this change there is no functional issue, as the code
> for switching the voltage in sdhci.c currently switches both, the
> VSELECT/SD_VSEL signal and the regulator voltage at the same time
> and doesn't run into an invalid corner case.
>
> We should still make sure, that we always use the correct register
> when controlling the regulator. At least in U-Boot this fixes an
> actual bug where the wrong IO voltage is used.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
> arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts | 6 +++---
> arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts | 6 +++---
> arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi | 1 +
> arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi | 1 +
> 4 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
> index 8b16bd68576c..bdcd9cd843c7 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
> @@ -344,7 +344,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
>
> @@ -357,7 +357,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
>
> @@ -370,7 +370,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
> };
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
> index a079322a3793..ba2a4491cf31 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
> @@ -321,7 +321,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
>
> @@ -334,7 +334,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
>
> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
The VSELECT pin should be driven by the (u)sdhc core...
> >;
> };
> };
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> index 5172883717d1..90daaf54e704 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
> regulator-name = "NVCC_SD (LDO5)";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <3300000>;
> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
muxed as GPIO, which is not the case. So I think that u-boot have a bug
within the (u)sdhc core.
Regards,
Marco
> };
> };
> };
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
> index 1f8326613ee9..7468a8aa771d 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
> @@ -195,6 +195,7 @@ reg_nvcc_sd: LDO5 {
> regulator-name = "NVCC_SD (LDO5)";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <3300000>;
> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> };
> };
> };
> --
> 2.39.1
>
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-13 16:15 ` Marco Felsch
0 siblings, 0 replies; 45+ messages in thread
From: Marco Felsch @ 2023-02-13 16:15 UTC (permalink / raw)
To: Frieder Schrempf
Cc: devicetree, Krzysztof Kozlowski, linux-arm-kernel, linux-kernel,
Rob Herring, Sascha Hauer, Shawn Guo, Marek Vasut,
Frieder Schrempf, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery,
Fabio Estevam
Hi Frieder,
thanks for the patch.
On 23-02-13, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> This fixes the LDO5 regulator handling of the pca9450 driver by
> taking the status of the SD_VSEL into account to determine which
> configuration register is used for the voltage setting.
>
> Even without this change there is no functional issue, as the code
> for switching the voltage in sdhci.c currently switches both, the
> VSELECT/SD_VSEL signal and the regulator voltage at the same time
> and doesn't run into an invalid corner case.
>
> We should still make sure, that we always use the correct register
> when controlling the regulator. At least in U-Boot this fixes an
> actual bug where the wrong IO voltage is used.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
> arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts | 6 +++---
> arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts | 6 +++---
> arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi | 1 +
> arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi | 1 +
> 4 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
> index 8b16bd68576c..bdcd9cd843c7 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
> @@ -344,7 +344,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
>
> @@ -357,7 +357,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
>
> @@ -370,7 +370,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
> };
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
> index a079322a3793..ba2a4491cf31 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
> @@ -321,7 +321,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
>
> @@ -334,7 +334,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >;
> };
>
> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
The VSELECT pin should be driven by the (u)sdhc core...
> >;
> };
> };
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> index 5172883717d1..90daaf54e704 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
> regulator-name = "NVCC_SD (LDO5)";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <3300000>;
> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
muxed as GPIO, which is not the case. So I think that u-boot have a bug
within the (u)sdhc core.
Regards,
Marco
> };
> };
> };
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
> index 1f8326613ee9..7468a8aa771d 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi
> @@ -195,6 +195,7 @@ reg_nvcc_sd: LDO5 {
> regulator-name = "NVCC_SD (LDO5)";
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <3300000>;
> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> };
> };
> };
> --
> 2.39.1
>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 3/6] Revert "regulator: pca9450: Add SD_VSEL GPIO for LDO5"
2023-02-13 15:58 ` [PATCH 3/6] Revert "regulator: pca9450: Add SD_VSEL GPIO for LDO5" Frieder Schrempf
@ 2023-02-13 16:16 ` Marek Vasut
2023-02-13 16:23 ` Frieder Schrempf
0 siblings, 1 reply; 45+ messages in thread
From: Marek Vasut @ 2023-02-13 16:16 UTC (permalink / raw)
To: Frieder Schrempf, Liam Girdwood, linux-kernel, Mark Brown
Cc: Frieder Schrempf, Per-Daniel Olsson, Rickard x Andersson,
Uwe Kleine-König
On 2/13/23 16:58, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> This reverts commit 8c67a11bae889f51fe5054364c3c789dfae3ad73.
>
> It turns out that all boards using the PCA9450 actually have the
> SD_VSEL input conencted to the VSELECT signal of the SoCs SD/MMC
'connected' , typo .
btw would it make sense to squash 3..5 patches into a single patch ?
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 16:15 ` Marco Felsch
@ 2023-02-13 16:18 ` Frieder Schrempf
-1 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 16:18 UTC (permalink / raw)
To: Marco Felsch, Frieder Schrempf
Cc: devicetree, Krzysztof Kozlowski, linux-arm-kernel, linux-kernel,
Rob Herring, Sascha Hauer, Shawn Guo, Marek Vasut,
Oleksij Rempel, Krzysztof Kozlowski, NXP Linux Team,
Pengutronix Kernel Team, Heiko Thiery, Fabio Estevam
On 13.02.23 17:15, Marco Felsch wrote:
> Hi Frieder,
>
> thanks for the patch.
>
> On 23-02-13, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> This fixes the LDO5 regulator handling of the pca9450 driver by
>> taking the status of the SD_VSEL into account to determine which
>> configuration register is used for the voltage setting.
>>
>> Even without this change there is no functional issue, as the code
>> for switching the voltage in sdhci.c currently switches both, the
>> VSELECT/SD_VSEL signal and the regulator voltage at the same time
>> and doesn't run into an invalid corner case.
>>
>> We should still make sure, that we always use the correct register
>> when controlling the regulator. At least in U-Boot this fixes an
>> actual bug where the wrong IO voltage is used.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>> ---
>> arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts | 6 +++---
>> arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts | 6 +++---
>> arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi | 1 +
>> arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi | 1 +
>> 4 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
>> index 8b16bd68576c..bdcd9cd843c7 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
>> @@ -344,7 +344,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>>
>> @@ -357,7 +357,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>>
>> @@ -370,7 +370,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>> };
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
>> index a079322a3793..ba2a4491cf31 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
>> @@ -321,7 +321,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>>
>> @@ -334,7 +334,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>>
>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>
> The VSELECT pin should be driven by the (u)sdhc core...
>
>> >;
>> };
>> };
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> index 5172883717d1..90daaf54e704 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
>> regulator-name = "NVCC_SD (LDO5)";
>> regulator-min-microvolt = <1800000>;
>> regulator-max-microvolt = <3300000>;
>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>
> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> muxed as GPIO, which is not the case. So I think that u-boot have a bug
> within the (u)sdhc core.
No, we don't mux the signal as GPIO. We still use the VSLECT mux option,
but enable the SION bit, so we can read back the current state of the
VSELECT signal via the GPIO.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-13 16:18 ` Frieder Schrempf
0 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 16:18 UTC (permalink / raw)
To: Marco Felsch, Frieder Schrempf
Cc: devicetree, Krzysztof Kozlowski, linux-arm-kernel, linux-kernel,
Rob Herring, Sascha Hauer, Shawn Guo, Marek Vasut,
Oleksij Rempel, Krzysztof Kozlowski, NXP Linux Team,
Pengutronix Kernel Team, Heiko Thiery, Fabio Estevam
On 13.02.23 17:15, Marco Felsch wrote:
> Hi Frieder,
>
> thanks for the patch.
>
> On 23-02-13, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> This fixes the LDO5 regulator handling of the pca9450 driver by
>> taking the status of the SD_VSEL into account to determine which
>> configuration register is used for the voltage setting.
>>
>> Even without this change there is no functional issue, as the code
>> for switching the voltage in sdhci.c currently switches both, the
>> VSELECT/SD_VSEL signal and the regulator voltage at the same time
>> and doesn't run into an invalid corner case.
>>
>> We should still make sure, that we always use the correct register
>> when controlling the regulator. At least in U-Boot this fixes an
>> actual bug where the wrong IO voltage is used.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>> ---
>> arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts | 6 +++---
>> arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts | 6 +++---
>> arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi | 1 +
>> arch/arm64/boot/dts/freescale/imx8mm-kontron-sl.dtsi | 1 +
>> 4 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
>> index 8b16bd68576c..bdcd9cd843c7 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl-osm-s.dts
>> @@ -344,7 +344,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>>
>> @@ -357,7 +357,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>>
>> @@ -370,7 +370,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>> };
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
>> index a079322a3793..ba2a4491cf31 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-bl.dts
>> @@ -321,7 +321,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d0
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d0
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d0
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>>
>> @@ -334,7 +334,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d4
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d4
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d4
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>> >;
>> };
>>
>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>
> The VSELECT pin should be driven by the (u)sdhc core...
>
>> >;
>> };
>> };
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> index 5172883717d1..90daaf54e704 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
>> regulator-name = "NVCC_SD (LDO5)";
>> regulator-min-microvolt = <1800000>;
>> regulator-max-microvolt = <3300000>;
>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>
> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> muxed as GPIO, which is not the case. So I think that u-boot have a bug
> within the (u)sdhc core.
No, we don't mux the signal as GPIO. We still use the VSLECT mux option,
but enable the SION bit, so we can read back the current state of the
VSELECT signal via the GPIO.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 3/6] Revert "regulator: pca9450: Add SD_VSEL GPIO for LDO5"
2023-02-13 16:16 ` Marek Vasut
@ 2023-02-13 16:23 ` Frieder Schrempf
2023-02-13 18:10 ` Marek Vasut
0 siblings, 1 reply; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-13 16:23 UTC (permalink / raw)
To: Marek Vasut, Frieder Schrempf, Liam Girdwood, linux-kernel, Mark Brown
Cc: Per-Daniel Olsson, Rickard x Andersson, Uwe Kleine-König
On 13.02.23 17:16, Marek Vasut wrote:
> On 2/13/23 16:58, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> This reverts commit 8c67a11bae889f51fe5054364c3c789dfae3ad73.
>>
>> It turns out that all boards using the PCA9450 actually have the
>> SD_VSEL input conencted to the VSELECT signal of the SoCs SD/MMC
>
> 'connected' , typo .
Thanks!
> btw would it make sense to squash 3..5 patches into a single patch ?
Hm, don't know. I think the changes are easier to understand with the
current separation between "revert", "core changes" and "pca9450 changes".
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 3/6] Revert "regulator: pca9450: Add SD_VSEL GPIO for LDO5"
2023-02-13 16:23 ` Frieder Schrempf
@ 2023-02-13 18:10 ` Marek Vasut
0 siblings, 0 replies; 45+ messages in thread
From: Marek Vasut @ 2023-02-13 18:10 UTC (permalink / raw)
To: Frieder Schrempf, Frieder Schrempf, Liam Girdwood, linux-kernel,
Mark Brown
Cc: Per-Daniel Olsson, Rickard x Andersson, Uwe Kleine-König
On 2/13/23 17:23, Frieder Schrempf wrote:
> On 13.02.23 17:16, Marek Vasut wrote:
>> On 2/13/23 16:58, Frieder Schrempf wrote:
>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>
>>> This reverts commit 8c67a11bae889f51fe5054364c3c789dfae3ad73.
>>>
>>> It turns out that all boards using the PCA9450 actually have the
>>> SD_VSEL input conencted to the VSELECT signal of the SoCs SD/MMC
>>
>> 'connected' , typo .
>
> Thanks!
>
>> btw would it make sense to squash 3..5 patches into a single patch ?
>
> Hm, don't know. I think the changes are easier to understand with the
> current separation between "revert", "core changes" and "pca9450 changes".
OK, I don't mind either way.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 16:15 ` Marco Felsch
@ 2023-02-13 18:12 ` Marek Vasut
-1 siblings, 0 replies; 45+ messages in thread
From: Marek Vasut @ 2023-02-13 18:12 UTC (permalink / raw)
To: Marco Felsch, Frieder Schrempf
Cc: devicetree, Krzysztof Kozlowski, linux-arm-kernel, linux-kernel,
Rob Herring, Sascha Hauer, Shawn Guo, Frieder Schrempf,
Oleksij Rempel, Krzysztof Kozlowski, NXP Linux Team,
Pengutronix Kernel Team, Heiko Thiery, Fabio Estevam
On 2/13/23 17:15, Marco Felsch wrote:
[...]
>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>
> The VSELECT pin should be driven by the (u)sdhc core...
>
>> >;
>> };
>> };
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> index 5172883717d1..90daaf54e704 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
>> regulator-name = "NVCC_SD (LDO5)";
>> regulator-min-microvolt = <1800000>;
>> regulator-max-microvolt = <3300000>;
>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>
> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> muxed as GPIO, which is not the case. So I think that u-boot have a bug
> within the (u)sdhc core.
The trick here is that the VSELECT is operated by the usdhc block as a
function pin, but the PMIC driver can read the current state of the
VSELECT pin by reading out the GPIO block SR register. Since the IOMUX
SION bit is set on the VSELECT pin, the state of the pin is reflected in
the GPIO block SR register even if the pin is muxed as function pin.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-13 18:12 ` Marek Vasut
0 siblings, 0 replies; 45+ messages in thread
From: Marek Vasut @ 2023-02-13 18:12 UTC (permalink / raw)
To: Marco Felsch, Frieder Schrempf
Cc: devicetree, Krzysztof Kozlowski, linux-arm-kernel, linux-kernel,
Rob Herring, Sascha Hauer, Shawn Guo, Frieder Schrempf,
Oleksij Rempel, Krzysztof Kozlowski, NXP Linux Team,
Pengutronix Kernel Team, Heiko Thiery, Fabio Estevam
On 2/13/23 17:15, Marco Felsch wrote:
[...]
>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>
> The VSELECT pin should be driven by the (u)sdhc core...
>
>> >;
>> };
>> };
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> index 5172883717d1..90daaf54e704 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
>> regulator-name = "NVCC_SD (LDO5)";
>> regulator-min-microvolt = <1800000>;
>> regulator-max-microvolt = <3300000>;
>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>
> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> muxed as GPIO, which is not the case. So I think that u-boot have a bug
> within the (u)sdhc core.
The trick here is that the VSELECT is operated by the usdhc block as a
function pin, but the PMIC driver can read the current state of the
VSELECT pin by reading out the GPIO block SR register. Since the IOMUX
SION bit is set on the VSELECT pin, the state of the pin is reflected in
the GPIO block SR register even if the pin is muxed as function pin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 18:12 ` Marek Vasut
@ 2023-02-13 19:56 ` Marco Felsch
-1 siblings, 0 replies; 45+ messages in thread
From: Marco Felsch @ 2023-02-13 19:56 UTC (permalink / raw)
To: Marek Vasut
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Frieder Schrempf, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery,
Fabio Estevam
Hi Marek, Frieder,
On 23-02-13, Marek Vasut wrote:
> On 2/13/23 17:15, Marco Felsch wrote:
>
> [...]
>
> > > @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> > > MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> > > MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> > > MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> > > - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> > > + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >
> > The VSELECT pin should be driven by the (u)sdhc core...
> >
> > > >;
> > > };
> > > };
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > index 5172883717d1..90daaf54e704 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
> > > regulator-name = "NVCC_SD (LDO5)";
> > > regulator-min-microvolt = <1800000>;
> > > regulator-max-microvolt = <3300000>;
> > > + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> >
> > and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> > muxed as GPIO, which is not the case. So I think that u-boot have a bug
> > within the (u)sdhc core.
>
> The trick here is that the VSELECT is operated by the usdhc block as a
> function pin, but the PMIC driver can read the current state of the VSELECT
> pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
> set on the VSELECT pin, the state of the pin is reflected in the GPIO block
> SR register even if the pin is muxed as function pin.
>
Thanks for this explanation :) Why does the regulator driver need to
know the current state of this pin? Since the voltage switching requires
some cmd's before the actual voltage level switch. So this must be
handled within the core.
Also after checking the driver, adding the sd-vsel-gpios will request
the specified gpio as output-high. Out of curiosity, what's the bug you
triggering within U-Boot?
Regards,
Marco
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-13 19:56 ` Marco Felsch
0 siblings, 0 replies; 45+ messages in thread
From: Marco Felsch @ 2023-02-13 19:56 UTC (permalink / raw)
To: Marek Vasut
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Frieder Schrempf, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery,
Fabio Estevam
Hi Marek, Frieder,
On 23-02-13, Marek Vasut wrote:
> On 2/13/23 17:15, Marco Felsch wrote:
>
> [...]
>
> > > @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> > > MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> > > MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> > > MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> > > - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> > > + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >
> > The VSELECT pin should be driven by the (u)sdhc core...
> >
> > > >;
> > > };
> > > };
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > index 5172883717d1..90daaf54e704 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
> > > regulator-name = "NVCC_SD (LDO5)";
> > > regulator-min-microvolt = <1800000>;
> > > regulator-max-microvolt = <3300000>;
> > > + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> >
> > and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> > muxed as GPIO, which is not the case. So I think that u-boot have a bug
> > within the (u)sdhc core.
>
> The trick here is that the VSELECT is operated by the usdhc block as a
> function pin, but the PMIC driver can read the current state of the VSELECT
> pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
> set on the VSELECT pin, the state of the pin is reflected in the GPIO block
> SR register even if the pin is muxed as function pin.
>
Thanks for this explanation :) Why does the regulator driver need to
know the current state of this pin? Since the voltage switching requires
some cmd's before the actual voltage level switch. So this must be
handled within the core.
Also after checking the driver, adding the sd-vsel-gpios will request
the specified gpio as output-high. Out of curiosity, what's the bug you
triggering within U-Boot?
Regards,
Marco
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 19:56 ` Marco Felsch
@ 2023-02-13 20:59 ` Marek Vasut
-1 siblings, 0 replies; 45+ messages in thread
From: Marek Vasut @ 2023-02-13 20:59 UTC (permalink / raw)
To: Marco Felsch
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Frieder Schrempf, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery,
Fabio Estevam
On 2/13/23 20:56, Marco Felsch wrote:
> Hi Marek, Frieder,
Hi,
> On 23-02-13, Marek Vasut wrote:
>> On 2/13/23 17:15, Marco Felsch wrote:
>>
>> [...]
>>
>>>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>>>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>>>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>>>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>>>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>>>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>>>
>>> The VSELECT pin should be driven by the (u)sdhc core...
>>>
>>>> >;
>>>> };
>>>> };
>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>> index 5172883717d1..90daaf54e704 100644
>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
>>>> regulator-name = "NVCC_SD (LDO5)";
>>>> regulator-min-microvolt = <1800000>;
>>>> regulator-max-microvolt = <3300000>;
>>>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>>>
>>> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
>>> muxed as GPIO, which is not the case. So I think that u-boot have a bug
>>> within the (u)sdhc core.
>>
>> The trick here is that the VSELECT is operated by the usdhc block as a
>> function pin, but the PMIC driver can read the current state of the VSELECT
>> pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
>> set on the VSELECT pin, the state of the pin is reflected in the GPIO block
>> SR register even if the pin is muxed as function pin.
>>
>
> Thanks for this explanation :) Why does the regulator driver need to
> know the current state of this pin?
Because that regulator has an input pin which selects between two states
of that regulator, L and H, and whatever L or H is depends on what is
configured into the regulator via I2C. To correctly report the state of
the regulator, you have to know the state of that input (selector) pin.
> Since the voltage switching requires
> some cmd's before the actual voltage level switch. So this must be
> handled within the core.
>
> Also after checking the driver, adding the sd-vsel-gpios will request
> the specified gpio as output-high.
The GPIO would have to be requested as input, obviously.
> Out of curiosity, what's the bug you
> triggering within U-Boot?
AFAICT the readback of the initial state of the regulator (see paragraph
above), which affects Linux all the same.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-13 20:59 ` Marek Vasut
0 siblings, 0 replies; 45+ messages in thread
From: Marek Vasut @ 2023-02-13 20:59 UTC (permalink / raw)
To: Marco Felsch
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Frieder Schrempf, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery,
Fabio Estevam
On 2/13/23 20:56, Marco Felsch wrote:
> Hi Marek, Frieder,
Hi,
> On 23-02-13, Marek Vasut wrote:
>> On 2/13/23 17:15, Marco Felsch wrote:
>>
>> [...]
>>
>>>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>>>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>>>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>>>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>>>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>>>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>>>
>>> The VSELECT pin should be driven by the (u)sdhc core...
>>>
>>>> >;
>>>> };
>>>> };
>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>> index 5172883717d1..90daaf54e704 100644
>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
>>>> regulator-name = "NVCC_SD (LDO5)";
>>>> regulator-min-microvolt = <1800000>;
>>>> regulator-max-microvolt = <3300000>;
>>>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>>>
>>> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
>>> muxed as GPIO, which is not the case. So I think that u-boot have a bug
>>> within the (u)sdhc core.
>>
>> The trick here is that the VSELECT is operated by the usdhc block as a
>> function pin, but the PMIC driver can read the current state of the VSELECT
>> pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
>> set on the VSELECT pin, the state of the pin is reflected in the GPIO block
>> SR register even if the pin is muxed as function pin.
>>
>
> Thanks for this explanation :) Why does the regulator driver need to
> know the current state of this pin?
Because that regulator has an input pin which selects between two states
of that regulator, L and H, and whatever L or H is depends on what is
configured into the regulator via I2C. To correctly report the state of
the regulator, you have to know the state of that input (selector) pin.
> Since the voltage switching requires
> some cmd's before the actual voltage level switch. So this must be
> handled within the core.
>
> Also after checking the driver, adding the sd-vsel-gpios will request
> the specified gpio as output-high.
The GPIO would have to be requested as input, obviously.
> Out of curiosity, what's the bug you
> triggering within U-Boot?
AFAICT the readback of the initial state of the regulator (see paragraph
above), which affects Linux all the same.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 16:18 ` Frieder Schrempf
@ 2023-02-13 21:02 ` Fabio Estevam
-1 siblings, 0 replies; 45+ messages in thread
From: Fabio Estevam @ 2023-02-13 21:02 UTC (permalink / raw)
To: Frieder Schrempf
Cc: Marco Felsch, Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Marek Vasut, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery
Hi Frieder,
On Mon, Feb 13, 2023 at 1:19 PM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
> No, we don't mux the signal as GPIO. We still use the VSLECT mux option,
> but enable the SION bit, so we can read back the current state of the
> VSELECT signal via the GPIO.
Please include this explanation in the commit log.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-13 21:02 ` Fabio Estevam
0 siblings, 0 replies; 45+ messages in thread
From: Fabio Estevam @ 2023-02-13 21:02 UTC (permalink / raw)
To: Frieder Schrempf
Cc: Marco Felsch, Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Marek Vasut, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery
Hi Frieder,
On Mon, Feb 13, 2023 at 1:19 PM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
> No, we don't mux the signal as GPIO. We still use the VSLECT mux option,
> but enable the SION bit, so we can read back the current state of the
> VSELECT signal via the GPIO.
Please include this explanation in the commit log.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 4/6] regulator: Add operation to let drivers select vsel register
2023-02-13 15:58 ` [PATCH 4/6] regulator: Add operation to let drivers select vsel register Frieder Schrempf
@ 2023-02-14 1:40 ` kernel test robot
2023-02-14 9:19 ` kernel test robot
2023-02-14 21:06 ` Mark Brown
2 siblings, 0 replies; 45+ messages in thread
From: kernel test robot @ 2023-02-14 1:40 UTC (permalink / raw)
To: Frieder Schrempf, Liam Girdwood, linux-kernel, Mark Brown
Cc: oe-kbuild-all, Marek Vasut, Frieder Schrempf, ChiYuan Huang,
Mauro Carvalho Chehab, linux-media
Hi Frieder,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on broonie-regulator/for-next]
[also build test WARNING on shawnguo/for-next arm/for-next arm/fixes arm64/for-next/core kvmarm/next rockchip/for-next soc/for-next xilinx-xlnx/master linus/master v6.2-rc8 next-20230213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Frieder-Schrempf/dt-bindings-regulator-pca9450-Document-new-usage-of-sd-vsel-gpios/20230214-013045
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
patch link: https://lore.kernel.org/r/20230213155833.1644366-5-frieder%40fris.de
patch subject: [PATCH 4/6] regulator: Add operation to let drivers select vsel register
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230214/202302140905.36f6bYXV-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/b76ab45ee4b60334c27d870b6d744a937ff94636
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Frieder-Schrempf/dt-bindings-regulator-pca9450-Document-new-usage-of-sd-vsel-gpios/20230214-013045
git checkout b76ab45ee4b60334c27d870b6d744a937ff94636
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/regulator/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302140905.36f6bYXV-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/regulator/helpers.c:226:14: warning: no previous prototype for 'regulator_get_hwreg_voltage_sel_regmap' [-Wmissing-prototypes]
226 | unsigned int regulator_get_hwreg_voltage_sel_regmap(struct regulator_dev *rdev)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/regulator_get_hwreg_voltage_sel_regmap +226 drivers/regulator/helpers.c
225
> 226 unsigned int regulator_get_hwreg_voltage_sel_regmap(struct regulator_dev *rdev)
227 {
228 const struct regulator_ops *ops = rdev->desc->ops;
229
230 if (ops->get_reg_voltage_sel)
231 return ops->get_reg_voltage_sel(rdev);
232
233 return rdev->desc->vsel_reg;
234 }
235
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-13 20:59 ` Marek Vasut
@ 2023-02-14 8:10 ` Marco Felsch
-1 siblings, 0 replies; 45+ messages in thread
From: Marco Felsch @ 2023-02-14 8:10 UTC (permalink / raw)
To: Marek Vasut
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Frieder Schrempf, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery,
Fabio Estevam
On 23-02-13, Marek Vasut wrote:
> On 2/13/23 20:56, Marco Felsch wrote:
> > Hi Marek, Frieder,
>
> Hi,
>
> > On 23-02-13, Marek Vasut wrote:
> > > On 2/13/23 17:15, Marco Felsch wrote:
> > >
> > > [...]
> > >
> > > > > @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> > > > > MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> > > > > MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> > > > > MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> > > > > - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> > > > > + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> > > >
> > > > The VSELECT pin should be driven by the (u)sdhc core...
> > > >
> > > > > >;
> > > > > };
> > > > > };
> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > > > index 5172883717d1..90daaf54e704 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > > > @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
> > > > > regulator-name = "NVCC_SD (LDO5)";
> > > > > regulator-min-microvolt = <1800000>;
> > > > > regulator-max-microvolt = <3300000>;
> > > > > + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> > > >
> > > > and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> > > > muxed as GPIO, which is not the case. So I think that u-boot have a bug
> > > > within the (u)sdhc core.
> > >
> > > The trick here is that the VSELECT is operated by the usdhc block as a
> > > function pin, but the PMIC driver can read the current state of the VSELECT
> > > pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
> > > set on the VSELECT pin, the state of the pin is reflected in the GPIO block
> > > SR register even if the pin is muxed as function pin.
> > >
> >
> > Thanks for this explanation :) Why does the regulator driver need to
> > know the current state of this pin?
>
> Because that regulator has an input pin which selects between two states of
> that regulator, L and H, and whatever L or H is depends on what is
> configured into the regulator via I2C. To correctly report the state of the
> regulator, you have to know the state of that input (selector) pin.
>
> > Since the voltage switching requires
> > some cmd's before the actual voltage level switch. So this must be
> > handled within the core.
> >
> > Also after checking the driver, adding the sd-vsel-gpios will request
> > the specified gpio as output-high.
>
> The GPIO would have to be requested as input, obviously.
But that isn't the case. According the driver comment they just want to
make sure that this GPIO is high to ensure that the correct regulator
config registers are used.
> > Out of curiosity, what's the bug you
> > triggering within U-Boot?
>
> AFAICT the readback of the initial state of the regulator (see paragraph
> above), which affects Linux all the same.
According the binding the driver should check this and apply the value
to the corresponding L/H register but unfortunately this isn't the case
yet. Does U-Boot handle this correctly?
Regards,
Marco
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-14 8:10 ` Marco Felsch
0 siblings, 0 replies; 45+ messages in thread
From: Marco Felsch @ 2023-02-14 8:10 UTC (permalink / raw)
To: Marek Vasut
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Frieder Schrempf, Oleksij Rempel, Krzysztof Kozlowski,
NXP Linux Team, Pengutronix Kernel Team, Heiko Thiery,
Fabio Estevam
On 23-02-13, Marek Vasut wrote:
> On 2/13/23 20:56, Marco Felsch wrote:
> > Hi Marek, Frieder,
>
> Hi,
>
> > On 23-02-13, Marek Vasut wrote:
> > > On 2/13/23 17:15, Marco Felsch wrote:
> > >
> > > [...]
> > >
> > > > > @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> > > > > MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> > > > > MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> > > > > MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> > > > > - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> > > > > + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> > > >
> > > > The VSELECT pin should be driven by the (u)sdhc core...
> > > >
> > > > > >;
> > > > > };
> > > > > };
> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > > > index 5172883717d1..90daaf54e704 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > > > @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
> > > > > regulator-name = "NVCC_SD (LDO5)";
> > > > > regulator-min-microvolt = <1800000>;
> > > > > regulator-max-microvolt = <3300000>;
> > > > > + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> > > >
> > > > and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> > > > muxed as GPIO, which is not the case. So I think that u-boot have a bug
> > > > within the (u)sdhc core.
> > >
> > > The trick here is that the VSELECT is operated by the usdhc block as a
> > > function pin, but the PMIC driver can read the current state of the VSELECT
> > > pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
> > > set on the VSELECT pin, the state of the pin is reflected in the GPIO block
> > > SR register even if the pin is muxed as function pin.
> > >
> >
> > Thanks for this explanation :) Why does the regulator driver need to
> > know the current state of this pin?
>
> Because that regulator has an input pin which selects between two states of
> that regulator, L and H, and whatever L or H is depends on what is
> configured into the regulator via I2C. To correctly report the state of the
> regulator, you have to know the state of that input (selector) pin.
>
> > Since the voltage switching requires
> > some cmd's before the actual voltage level switch. So this must be
> > handled within the core.
> >
> > Also after checking the driver, adding the sd-vsel-gpios will request
> > the specified gpio as output-high.
>
> The GPIO would have to be requested as input, obviously.
But that isn't the case. According the driver comment they just want to
make sure that this GPIO is high to ensure that the correct regulator
config registers are used.
> > Out of curiosity, what's the bug you
> > triggering within U-Boot?
>
> AFAICT the readback of the initial state of the regulator (see paragraph
> above), which affects Linux all the same.
According the binding the driver should check this and apply the value
to the corresponding L/H register but unfortunately this isn't the case
yet. Does U-Boot handle this correctly?
Regards,
Marco
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-14 8:10 ` Marco Felsch
@ 2023-02-14 8:26 ` Frieder Schrempf
-1 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-14 8:26 UTC (permalink / raw)
To: Marco Felsch, Marek Vasut
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Oleksij Rempel, Krzysztof Kozlowski, NXP Linux Team,
Pengutronix Kernel Team, Heiko Thiery, Fabio Estevam
Hi Marco,
On 14.02.23 09:10, Marco Felsch wrote:
> On 23-02-13, Marek Vasut wrote:
>> On 2/13/23 20:56, Marco Felsch wrote:
>>> Hi Marek, Frieder,
>>
>> Hi,
>>
>>> On 23-02-13, Marek Vasut wrote:
>>>> On 2/13/23 17:15, Marco Felsch wrote:
>>>>
>>>> [...]
>>>>
>>>>>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>>>>>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>>>>>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>>>>>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>>>>>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>>>>>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>>>>>
>>>>> The VSELECT pin should be driven by the (u)sdhc core...
>>>>>
>>>>>> >;
>>>>>> };
>>>>>> };
>>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>>>> index 5172883717d1..90daaf54e704 100644
>>>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>>>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
>>>>>> regulator-name = "NVCC_SD (LDO5)";
>>>>>> regulator-min-microvolt = <1800000>;
>>>>>> regulator-max-microvolt = <3300000>;
>>>>>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>>>>>
>>>>> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
>>>>> muxed as GPIO, which is not the case. So I think that u-boot have a bug
>>>>> within the (u)sdhc core.
>>>>
>>>> The trick here is that the VSELECT is operated by the usdhc block as a
>>>> function pin, but the PMIC driver can read the current state of the VSELECT
>>>> pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
>>>> set on the VSELECT pin, the state of the pin is reflected in the GPIO block
>>>> SR register even if the pin is muxed as function pin.
>>>>
>>>
>>> Thanks for this explanation :) Why does the regulator driver need to
>>> know the current state of this pin?
>>
>> Because that regulator has an input pin which selects between two states of
>> that regulator, L and H, and whatever L or H is depends on what is
>> configured into the regulator via I2C. To correctly report the state of the
>> regulator, you have to know the state of that input (selector) pin.
>>
>>> Since the voltage switching requires
>>> some cmd's before the actual voltage level switch. So this must be
>>> handled within the core.
>>>
>>> Also after checking the driver, adding the sd-vsel-gpios will request
>>> the specified gpio as output-high.
>>
>> The GPIO would have to be requested as input, obviously.
>
> But that isn't the case. According the driver comment they just want to
> make sure that this GPIO is high to ensure that the correct regulator
> config registers are used.
It seems like you look at the wrong code. We previously had the
sd-vsel-gpios used as you describe, as an output set to a fixed high
level to make sure that the SD_VSEL state matches the driver using the H
register. This code is reverted in patch 3 and patch 5 implements the
sd-vsel-gpios as input as described by Marek. See:
https://lore.kernel.org/lkml/20230213155833.1644366-4-frieder@fris.de/
https://lore.kernel.org/lkml/20230213155833.1644366-6-frieder@fris.de/
Sorry, my scripts didn't cc everyone for all patches, which is probably
why you missed these changes.
>
>>> Out of curiosity, what's the bug you
>>> triggering within U-Boot?
>>
>> AFAICT the readback of the initial state of the regulator (see paragraph
>> above), which affects Linux all the same.
>
> According the binding the driver should check this and apply the value
> to the corresponding L/H register but unfortunately this isn't the case
> yet. Does U-Boot handle this correctly?
See above.
Thanks
Frieder
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-14 8:26 ` Frieder Schrempf
0 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-14 8:26 UTC (permalink / raw)
To: Marco Felsch, Marek Vasut
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Oleksij Rempel, Krzysztof Kozlowski, NXP Linux Team,
Pengutronix Kernel Team, Heiko Thiery, Fabio Estevam
Hi Marco,
On 14.02.23 09:10, Marco Felsch wrote:
> On 23-02-13, Marek Vasut wrote:
>> On 2/13/23 20:56, Marco Felsch wrote:
>>> Hi Marek, Frieder,
>>
>> Hi,
>>
>>> On 23-02-13, Marek Vasut wrote:
>>>> On 2/13/23 17:15, Marco Felsch wrote:
>>>>
>>>> [...]
>>>>
>>>>>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
>>>>>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
>>>>>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
>>>>>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
>>>>>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
>>>>>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
>>>>>
>>>>> The VSELECT pin should be driven by the (u)sdhc core...
>>>>>
>>>>>> >;
>>>>>> };
>>>>>> };
>>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>>>> index 5172883717d1..90daaf54e704 100644
>>>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
>>>>>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
>>>>>> regulator-name = "NVCC_SD (LDO5)";
>>>>>> regulator-min-microvolt = <1800000>;
>>>>>> regulator-max-microvolt = <3300000>;
>>>>>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>>>>>
>>>>> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
>>>>> muxed as GPIO, which is not the case. So I think that u-boot have a bug
>>>>> within the (u)sdhc core.
>>>>
>>>> The trick here is that the VSELECT is operated by the usdhc block as a
>>>> function pin, but the PMIC driver can read the current state of the VSELECT
>>>> pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
>>>> set on the VSELECT pin, the state of the pin is reflected in the GPIO block
>>>> SR register even if the pin is muxed as function pin.
>>>>
>>>
>>> Thanks for this explanation :) Why does the regulator driver need to
>>> know the current state of this pin?
>>
>> Because that regulator has an input pin which selects between two states of
>> that regulator, L and H, and whatever L or H is depends on what is
>> configured into the regulator via I2C. To correctly report the state of the
>> regulator, you have to know the state of that input (selector) pin.
>>
>>> Since the voltage switching requires
>>> some cmd's before the actual voltage level switch. So this must be
>>> handled within the core.
>>>
>>> Also after checking the driver, adding the sd-vsel-gpios will request
>>> the specified gpio as output-high.
>>
>> The GPIO would have to be requested as input, obviously.
>
> But that isn't the case. According the driver comment they just want to
> make sure that this GPIO is high to ensure that the correct regulator
> config registers are used.
It seems like you look at the wrong code. We previously had the
sd-vsel-gpios used as you describe, as an output set to a fixed high
level to make sure that the SD_VSEL state matches the driver using the H
register. This code is reverted in patch 3 and patch 5 implements the
sd-vsel-gpios as input as described by Marek. See:
https://lore.kernel.org/lkml/20230213155833.1644366-4-frieder@fris.de/
https://lore.kernel.org/lkml/20230213155833.1644366-6-frieder@fris.de/
Sorry, my scripts didn't cc everyone for all patches, which is probably
why you missed these changes.
>
>>> Out of curiosity, what's the bug you
>>> triggering within U-Boot?
>>
>> AFAICT the readback of the initial state of the regulator (see paragraph
>> above), which affects Linux all the same.
>
> According the binding the driver should check this and apply the value
> to the corresponding L/H register but unfortunately this isn't the case
> yet. Does U-Boot handle this correctly?
See above.
Thanks
Frieder
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 4/6] regulator: Add operation to let drivers select vsel register
2023-02-13 15:58 ` [PATCH 4/6] regulator: Add operation to let drivers select vsel register Frieder Schrempf
2023-02-14 1:40 ` kernel test robot
@ 2023-02-14 9:19 ` kernel test robot
2023-02-14 21:06 ` Mark Brown
2 siblings, 0 replies; 45+ messages in thread
From: kernel test robot @ 2023-02-14 9:19 UTC (permalink / raw)
To: Frieder Schrempf, Liam Girdwood, linux-kernel, Mark Brown
Cc: llvm, oe-kbuild-all, Marek Vasut, Frieder Schrempf,
ChiYuan Huang, Mauro Carvalho Chehab, linux-media
Hi Frieder,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on broonie-regulator/for-next]
[also build test WARNING on shawnguo/for-next arm/for-next arm/fixes arm64/for-next/core kvmarm/next rockchip/for-next soc/for-next xilinx-xlnx/master linus/master v6.2-rc8 next-20230214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Frieder-Schrempf/dt-bindings-regulator-pca9450-Document-new-usage-of-sd-vsel-gpios/20230214-013045
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
patch link: https://lore.kernel.org/r/20230213155833.1644366-5-frieder%40fris.de
patch subject: [PATCH 4/6] regulator: Add operation to let drivers select vsel register
config: i386-randconfig-a003-20230213 (https://download.01.org/0day-ci/archive/20230214/202302141754.N3CvO9lF-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/b76ab45ee4b60334c27d870b6d744a937ff94636
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Frieder-Schrempf/dt-bindings-regulator-pca9450-Document-new-usage-of-sd-vsel-gpios/20230214-013045
git checkout b76ab45ee4b60334c27d870b6d744a937ff94636
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/regulator/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302141754.N3CvO9lF-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/regulator/helpers.c:226:14: warning: no previous prototype for function 'regulator_get_hwreg_voltage_sel_regmap' [-Wmissing-prototypes]
unsigned int regulator_get_hwreg_voltage_sel_regmap(struct regulator_dev *rdev)
^
drivers/regulator/helpers.c:226:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
unsigned int regulator_get_hwreg_voltage_sel_regmap(struct regulator_dev *rdev)
^
static
1 warning generated.
vim +/regulator_get_hwreg_voltage_sel_regmap +226 drivers/regulator/helpers.c
225
> 226 unsigned int regulator_get_hwreg_voltage_sel_regmap(struct regulator_dev *rdev)
227 {
228 const struct regulator_ops *ops = rdev->desc->ops;
229
230 if (ops->get_reg_voltage_sel)
231 return ops->get_reg_voltage_sel(rdev);
232
233 return rdev->desc->vsel_reg;
234 }
235
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
2023-02-14 8:26 ` Frieder Schrempf
@ 2023-02-14 11:46 ` Marco Felsch
-1 siblings, 0 replies; 45+ messages in thread
From: Marco Felsch @ 2023-02-14 11:46 UTC (permalink / raw)
To: Frieder Schrempf
Cc: Marek Vasut, Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Oleksij Rempel, Krzysztof Kozlowski, NXP Linux Team,
Pengutronix Kernel Team, Heiko Thiery, Fabio Estevam
On 23-02-14, Frieder Schrempf wrote:
> Hi Marco,
>
> On 14.02.23 09:10, Marco Felsch wrote:
> > On 23-02-13, Marek Vasut wrote:
> >> On 2/13/23 20:56, Marco Felsch wrote:
> >>> Hi Marek, Frieder,
> >>
> >> Hi,
> >>
> >>> On 23-02-13, Marek Vasut wrote:
> >>>> On 2/13/23 17:15, Marco Felsch wrote:
> >>>>
> >>>> [...]
> >>>>
> >>>>>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> >>>>>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> >>>>>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> >>>>>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> >>>>>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> >>>>>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >>>>>
> >>>>> The VSELECT pin should be driven by the (u)sdhc core...
> >>>>>
> >>>>>> >;
> >>>>>> };
> >>>>>> };
> >>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> >>>>>> index 5172883717d1..90daaf54e704 100644
> >>>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> >>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> >>>>>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
> >>>>>> regulator-name = "NVCC_SD (LDO5)";
> >>>>>> regulator-min-microvolt = <1800000>;
> >>>>>> regulator-max-microvolt = <3300000>;
> >>>>>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> >>>>>
> >>>>> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> >>>>> muxed as GPIO, which is not the case. So I think that u-boot have a bug
> >>>>> within the (u)sdhc core.
> >>>>
> >>>> The trick here is that the VSELECT is operated by the usdhc block as a
> >>>> function pin, but the PMIC driver can read the current state of the VSELECT
> >>>> pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
> >>>> set on the VSELECT pin, the state of the pin is reflected in the GPIO block
> >>>> SR register even if the pin is muxed as function pin.
> >>>>
> >>>
> >>> Thanks for this explanation :) Why does the regulator driver need to
> >>> know the current state of this pin?
> >>
> >> Because that regulator has an input pin which selects between two states of
> >> that regulator, L and H, and whatever L or H is depends on what is
> >> configured into the regulator via I2C. To correctly report the state of the
> >> regulator, you have to know the state of that input (selector) pin.
> >>
> >>> Since the voltage switching requires
> >>> some cmd's before the actual voltage level switch. So this must be
> >>> handled within the core.
> >>>
> >>> Also after checking the driver, adding the sd-vsel-gpios will request
> >>> the specified gpio as output-high.
> >>
> >> The GPIO would have to be requested as input, obviously.
> >
> > But that isn't the case. According the driver comment they just want to
> > make sure that this GPIO is high to ensure that the correct regulator
> > config registers are used.
>
> It seems like you look at the wrong code. We previously had the
> sd-vsel-gpios used as you describe, as an output set to a fixed high
> level to make sure that the SD_VSEL state matches the driver using the H
> register. This code is reverted in patch 3 and patch 5 implements the
> sd-vsel-gpios as input as described by Marek. See:
>
> https://lore.kernel.org/lkml/20230213155833.1644366-4-frieder@fris.de/
> https://lore.kernel.org/lkml/20230213155833.1644366-6-frieder@fris.de/
>
> Sorry, my scripts didn't cc everyone for all patches, which is probably
> why you missed these changes.
Ah okay this brings light into the dark :) Thanks for the pointers, just
received the DT changes.
Regards,
Marco
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
@ 2023-02-14 11:46 ` Marco Felsch
0 siblings, 0 replies; 45+ messages in thread
From: Marco Felsch @ 2023-02-14 11:46 UTC (permalink / raw)
To: Frieder Schrempf
Cc: Marek Vasut, Frieder Schrempf, devicetree, Krzysztof Kozlowski,
linux-arm-kernel, linux-kernel, Rob Herring, Sascha Hauer,
Shawn Guo, Oleksij Rempel, Krzysztof Kozlowski, NXP Linux Team,
Pengutronix Kernel Team, Heiko Thiery, Fabio Estevam
On 23-02-14, Frieder Schrempf wrote:
> Hi Marco,
>
> On 14.02.23 09:10, Marco Felsch wrote:
> > On 23-02-13, Marek Vasut wrote:
> >> On 2/13/23 20:56, Marco Felsch wrote:
> >>> Hi Marek, Frieder,
> >>
> >> Hi,
> >>
> >>> On 23-02-13, Marek Vasut wrote:
> >>>> On 2/13/23 17:15, Marco Felsch wrote:
> >>>>
> >>>> [...]
> >>>>
> >>>>>> @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1 0x1d6
> >>>>>> MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2 0x1d6
> >>>>>> MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3 0x1d6
> >>>>>> MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12 0x019
> >>>>>> - MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x1d0
> >>>>>> + MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT 0x400001d0
> >>>>>
> >>>>> The VSELECT pin should be driven by the (u)sdhc core...
> >>>>>
> >>>>>> >;
> >>>>>> };
> >>>>>> };
> >>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> >>>>>> index 5172883717d1..90daaf54e704 100644
> >>>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> >>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> >>>>>> @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
> >>>>>> regulator-name = "NVCC_SD (LDO5)";
> >>>>>> regulator-min-microvolt = <1800000>;
> >>>>>> regulator-max-microvolt = <3300000>;
> >>>>>> + sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> >>>>>
> >>>>> and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> >>>>> muxed as GPIO, which is not the case. So I think that u-boot have a bug
> >>>>> within the (u)sdhc core.
> >>>>
> >>>> The trick here is that the VSELECT is operated by the usdhc block as a
> >>>> function pin, but the PMIC driver can read the current state of the VSELECT
> >>>> pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
> >>>> set on the VSELECT pin, the state of the pin is reflected in the GPIO block
> >>>> SR register even if the pin is muxed as function pin.
> >>>>
> >>>
> >>> Thanks for this explanation :) Why does the regulator driver need to
> >>> know the current state of this pin?
> >>
> >> Because that regulator has an input pin which selects between two states of
> >> that regulator, L and H, and whatever L or H is depends on what is
> >> configured into the regulator via I2C. To correctly report the state of the
> >> regulator, you have to know the state of that input (selector) pin.
> >>
> >>> Since the voltage switching requires
> >>> some cmd's before the actual voltage level switch. So this must be
> >>> handled within the core.
> >>>
> >>> Also after checking the driver, adding the sd-vsel-gpios will request
> >>> the specified gpio as output-high.
> >>
> >> The GPIO would have to be requested as input, obviously.
> >
> > But that isn't the case. According the driver comment they just want to
> > make sure that this GPIO is high to ensure that the correct regulator
> > config registers are used.
>
> It seems like you look at the wrong code. We previously had the
> sd-vsel-gpios used as you describe, as an output set to a fixed high
> level to make sure that the SD_VSEL state matches the driver using the H
> register. This code is reverted in patch 3 and patch 5 implements the
> sd-vsel-gpios as input as described by Marek. See:
>
> https://lore.kernel.org/lkml/20230213155833.1644366-4-frieder@fris.de/
> https://lore.kernel.org/lkml/20230213155833.1644366-6-frieder@fris.de/
>
> Sorry, my scripts didn't cc everyone for all patches, which is probably
> why you missed these changes.
Ah okay this brings light into the dark :) Thanks for the pointers, just
received the DT changes.
Regards,
Marco
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 4/6] regulator: Add operation to let drivers select vsel register
2023-02-13 15:58 ` [PATCH 4/6] regulator: Add operation to let drivers select vsel register Frieder Schrempf
2023-02-14 1:40 ` kernel test robot
2023-02-14 9:19 ` kernel test robot
@ 2023-02-14 21:06 ` Mark Brown
2023-02-16 9:05 ` Frieder Schrempf
2 siblings, 1 reply; 45+ messages in thread
From: Mark Brown @ 2023-02-14 21:06 UTC (permalink / raw)
To: Frieder Schrempf
Cc: Liam Girdwood, linux-kernel, Marek Vasut, Frieder Schrempf,
ChiYuan Huang, Mauro Carvalho Chehab
[-- Attachment #1: Type: text/plain, Size: 946 bytes --]
On Mon, Feb 13, 2023 at 04:58:22PM +0100, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> There are regulators that use multiple registers for storing the
> voltage. Add a get_reg_voltage_sel member to struct regulator_ops in
> order to let drivers register a function that returns the currently
> used register.
>
> The pca9450 driver will be a user of this as the LDO5 regulator of
> that chip uses two different control registers depending on the
> state of an external signal.
Aside from the build warnings the bots reported it's not clear to
me that it's better to do this than it is to just have these
drivers implement appropriate ops directly - there's probably
going to be cases when it's a different bitfield in the same
register, and by the time you've implemented the op so things
aren't completely data driven I'm not sure how much you win by
reusing the register read/write.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/6] dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios
2023-02-13 15:58 ` [PATCH 1/6] dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios Frieder Schrempf
@ 2023-02-15 20:02 ` Rob Herring
2023-02-16 1:27 ` Marek Vasut
0 siblings, 1 reply; 45+ messages in thread
From: Rob Herring @ 2023-02-15 20:02 UTC (permalink / raw)
To: Frieder Schrempf
Cc: devicetree, Krzysztof Kozlowski, Liam Girdwood, linux-kernel,
Mark Brown, Robin Gong, Marek Vasut, Frieder Schrempf,
Per-Daniel Olsson, Rickard x Andersson
On Mon, Feb 13, 2023 at 04:58:19PM +0100, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> The sd-vsel-gpios property is abandoned in its current meaning as an
> output. We now use it to specify an optional signal that can be
> evaluated by the driver in order to retrieve the current status
> of the SD_VSEL signal that is used to select the control register
> of LDO5.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
> .../regulator/nxp,pca9450-regulator.yaml | 23 ++++++++++++++-----
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> index 835b53302db8..c86534538a4e 100644
> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> @@ -40,8 +40,24 @@ properties:
> description: |
> list of regulators provided by this controller
>
> + properties:
> + LDO5:
> + type: object
> + $ref: regulator.yaml#
> + description:
> + Properties for single LDO5 regulator.
> +
> + properties:
> + sd-vsel-gpios:
It is a pin on the device, right? Then it belongs in the device node as
it was.
Can't the direction of the signal tell you how it is used? Assuming the
pin is bidirectional?
The binding should support any possible way the device is wired, not
just what's been seen so far on some boards.
Rob
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/6] dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios
2023-02-15 20:02 ` Rob Herring
@ 2023-02-16 1:27 ` Marek Vasut
2023-02-16 2:30 ` Rob Herring
0 siblings, 1 reply; 45+ messages in thread
From: Marek Vasut @ 2023-02-16 1:27 UTC (permalink / raw)
To: Rob Herring, Frieder Schrempf
Cc: devicetree, Krzysztof Kozlowski, Liam Girdwood, linux-kernel,
Mark Brown, Robin Gong, Frieder Schrempf, Per-Daniel Olsson,
Rickard x Andersson
On 2/15/23 21:02, Rob Herring wrote:
> On Mon, Feb 13, 2023 at 04:58:19PM +0100, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> The sd-vsel-gpios property is abandoned in its current meaning as an
>> output. We now use it to specify an optional signal that can be
>> evaluated by the driver in order to retrieve the current status
>> of the SD_VSEL signal that is used to select the control register
>> of LDO5.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>> ---
>> .../regulator/nxp,pca9450-regulator.yaml | 23 ++++++++++++++-----
>> 1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> index 835b53302db8..c86534538a4e 100644
>> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> @@ -40,8 +40,24 @@ properties:
>> description: |
>> list of regulators provided by this controller
>>
>> + properties:
>> + LDO5:
>> + type: object
>> + $ref: regulator.yaml#
>> + description:
>> + Properties for single LDO5 regulator.
>> +
>> + properties:
>> + sd-vsel-gpios:
>
> It is a pin on the device, right? Then it belongs in the device node as
> it was.
>
> Can't the direction of the signal tell you how it is used? Assuming the
> pin is bidirectional?
The pin is input to the PMIC, it is unidirection, i.e.
SoC(output)---->(input)PMIC
> The binding should support any possible way the device is wired, not
> just what's been seen so far on some boards.
The usage is always the above as far as I can tell.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/6] dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios
2023-02-16 1:27 ` Marek Vasut
@ 2023-02-16 2:30 ` Rob Herring
2023-02-16 10:15 ` Frieder Schrempf
0 siblings, 1 reply; 45+ messages in thread
From: Rob Herring @ 2023-02-16 2:30 UTC (permalink / raw)
To: Marek Vasut
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski, Liam Girdwood,
linux-kernel, Mark Brown, Robin Gong, Frieder Schrempf,
Per-Daniel Olsson, Rickard x Andersson
On Wed, Feb 15, 2023 at 7:27 PM Marek Vasut <marex@denx.de> wrote:
>
> On 2/15/23 21:02, Rob Herring wrote:
> > On Mon, Feb 13, 2023 at 04:58:19PM +0100, Frieder Schrempf wrote:
> >> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> >>
> >> The sd-vsel-gpios property is abandoned in its current meaning as an
> >> output. We now use it to specify an optional signal that can be
> >> evaluated by the driver in order to retrieve the current status
> >> of the SD_VSEL signal that is used to select the control register
> >> of LDO5.
> >>
> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> >> ---
> >> .../regulator/nxp,pca9450-regulator.yaml | 23 ++++++++++++++-----
> >> 1 file changed, 17 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> >> index 835b53302db8..c86534538a4e 100644
> >> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> >> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> >> @@ -40,8 +40,24 @@ properties:
> >> description: |
> >> list of regulators provided by this controller
> >>
> >> + properties:
> >> + LDO5:
> >> + type: object
> >> + $ref: regulator.yaml#
> >> + description:
> >> + Properties for single LDO5 regulator.
> >> +
> >> + properties:
> >> + sd-vsel-gpios:
> >
> > It is a pin on the device, right? Then it belongs in the device node as
> > it was.
> >
> > Can't the direction of the signal tell you how it is used? Assuming the
> > pin is bidirectional?
>
> The pin is input to the PMIC, it is unidirection, i.e.
>
> SoC(output)---->(input)PMIC
>
> > The binding should support any possible way the device is wired, not
> > just what's been seen so far on some boards.
>
> The usage is always the above as far as I can tell.
This patch is saying the opposite though. Something else drives the
signal, but the signal is also routed to the SoC to sample the state.
Rob
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 4/6] regulator: Add operation to let drivers select vsel register
2023-02-14 21:06 ` Mark Brown
@ 2023-02-16 9:05 ` Frieder Schrempf
2023-02-16 14:31 ` Mark Brown
0 siblings, 1 reply; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-16 9:05 UTC (permalink / raw)
To: Mark Brown, Frieder Schrempf
Cc: Liam Girdwood, linux-kernel, Marek Vasut, ChiYuan Huang,
Mauro Carvalho Chehab
Hi Mark,
On 14.02.23 22:06, Mark Brown wrote:
> On Mon, Feb 13, 2023 at 04:58:22PM +0100, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> There are regulators that use multiple registers for storing the
>> voltage. Add a get_reg_voltage_sel member to struct regulator_ops in
>> order to let drivers register a function that returns the currently
>> used register.
>>
>> The pca9450 driver will be a user of this as the LDO5 regulator of
>> that chip uses two different control registers depending on the
>> state of an external signal.
>
> Aside from the build warnings the bots reported it's not clear to
> me that it's better to do this than it is to just have these
> drivers implement appropriate ops directly - there's probably
> going to be cases when it's a different bitfield in the same
> register, and by the time you've implemented the op so things
> aren't completely data driven I'm not sure how much you win by
> reusing the register read/write.
Thanks for the feedback. Makes sense to me, I can do that.
Just to be sure: you are suggesting to to leave the core untouched, use
the existing [get/set]_voltage() ops in the driver and reimplement the
logic of regulator_[get/set]_voltage_sel_regmap() there, right?
Thanks
Frieder
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/6] dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios
2023-02-16 2:30 ` Rob Herring
@ 2023-02-16 10:15 ` Frieder Schrempf
0 siblings, 0 replies; 45+ messages in thread
From: Frieder Schrempf @ 2023-02-16 10:15 UTC (permalink / raw)
To: Rob Herring, Marek Vasut
Cc: Frieder Schrempf, devicetree, Krzysztof Kozlowski, Liam Girdwood,
linux-kernel, Mark Brown, Robin Gong, Per-Daniel Olsson,
Rickard x Andersson
On 16.02.23 03:30, Rob Herring wrote:
> On Wed, Feb 15, 2023 at 7:27 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 2/15/23 21:02, Rob Herring wrote:
>>> On Mon, Feb 13, 2023 at 04:58:19PM +0100, Frieder Schrempf wrote:
>>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>>
>>>> The sd-vsel-gpios property is abandoned in its current meaning as an
>>>> output. We now use it to specify an optional signal that can be
>>>> evaluated by the driver in order to retrieve the current status
>>>> of the SD_VSEL signal that is used to select the control register
>>>> of LDO5.
>>>>
>>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>> ---
>>>> .../regulator/nxp,pca9450-regulator.yaml | 23 ++++++++++++++-----
>>>> 1 file changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>>> index 835b53302db8..c86534538a4e 100644
>>>> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>>> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>>> @@ -40,8 +40,24 @@ properties:
>>>> description: |
>>>> list of regulators provided by this controller
>>>>
>>>> + properties:
>>>> + LDO5:
>>>> + type: object
>>>> + $ref: regulator.yaml#
>>>> + description:
>>>> + Properties for single LDO5 regulator.
>>>> +
>>>> + properties:
>>>> + sd-vsel-gpios:
>>>
>>> It is a pin on the device, right? Then it belongs in the device node as
>>> it was.
Physically it's a pin on the PCA9450 chip. If you look at the block
diagram in the datasheet [1] (page 3) you can see though, that the
SD_VSEL signal is routed to the LD05 regulator block inside the chip.
This makes me think that the signal is best described inside the LDO5 node.
>>>
>>> Can't the direction of the signal tell you how it is used? Assuming the
>>> pin is bidirectional?
>>
>> The pin is input to the PMIC, it is unidirection, i.e.
>>
>> SoC(output)---->(input)PMIC
>>
>>> The binding should support any possible way the device is wired, not
>>> just what's been seen so far on some boards.
>>
>> The usage is always the above as far as I can tell.
There is only one usage that is likely to occur and that is the one we
describe here.
There are other ways to wire up the signal of course and in some
unlikely event a hardware engineer might have the idea to hard-wire the
SD_VSEL to a fixed level or wire it up to a SoC pin that doesn't have
the VSELECT mux option.
But I don't really see a good reason for covering these cases in the
binding/driver if there are good chances we won't ever need them.
> This patch is saying the opposite though. Something else drives the
> signal, but the signal is also routed to the SoC to sample the state.
SoC PMIC
+-----------------------+ +-------------------+
| | | |
| | | |
| GPIO <----------+ | | |
| | | SD_VSEL| +-------+ |
| USHC_VSELECT -->+------------------->| LDO5 | |
| | | +-------+ |
| | | |
+-----------------------+ +-------------------+
This is how the setup looks like. The SD_VSEL on the PMIC is always an
input. It's driven by the SoC's VSELECT signal (controlled by the USDHC
controller) and we use the SION bit in the IOMUX to internally loop back
the signal in order to sample it using the GPIO.
[1] https://www.nxp.com/docs/en/data-sheet/PCA9450DS.pdf
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 4/6] regulator: Add operation to let drivers select vsel register
2023-02-16 9:05 ` Frieder Schrempf
@ 2023-02-16 14:31 ` Mark Brown
0 siblings, 0 replies; 45+ messages in thread
From: Mark Brown @ 2023-02-16 14:31 UTC (permalink / raw)
To: Frieder Schrempf
Cc: Frieder Schrempf, Liam Girdwood, linux-kernel, Marek Vasut,
ChiYuan Huang, Mauro Carvalho Chehab
[-- Attachment #1: Type: text/plain, Size: 399 bytes --]
On Thu, Feb 16, 2023 at 10:05:49AM +0100, Frieder Schrempf wrote:
> Just to be sure: you are suggesting to to leave the core untouched, use
> the existing [get/set]_voltage() ops in the driver and reimplement the
> logic of regulator_[get/set]_voltage_sel_regmap() there, right?
Yes, exactly. If we get a lot of drivers doing this we can
always factor out later but it feels premature right now.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 5/6] regulator: pca9450: Fix control register for LDO5
@ 2023-02-14 21:52 kernel test robot
0 siblings, 0 replies; 45+ messages in thread
From: kernel test robot @ 2023-02-14 21:52 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230213155833.1644366-6-frieder@fris.de>
References: <20230213155833.1644366-6-frieder@fris.de>
TO: Frieder Schrempf <frieder@fris.de>
TO: Liam Girdwood <lgirdwood@gmail.com>
TO: linux-kernel@vger.kernel.org
TO: Mark Brown <broonie@kernel.org>
CC: Marek Vasut <marex@denx.de>
CC: Frieder Schrempf <frieder.schrempf@kontron.de>
CC: "Per-Daniel Olsson" <perdo@axis.com>
CC: Rickard x Andersson <rickaran@axis.com>
CC: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Hi Frieder,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on broonie-regulator/for-next]
[also build test WARNING on shawnguo/for-next arm/for-next arm/fixes arm64/for-next/core kvmarm/next rockchip/for-next soc/for-next xilinx-xlnx/master linus/master v6.2-rc8 next-20230214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Frieder-Schrempf/dt-bindings-regulator-pca9450-Document-new-usage-of-sd-vsel-gpios/20230214-013045
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
patch link: https://lore.kernel.org/r/20230213155833.1644366-6-frieder%40fris.de
patch subject: [PATCH 5/6] regulator: pca9450: Fix control register for LDO5
:::::: branch date: 28 hours ago
:::::: commit date: 28 hours ago
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230215/202302150540.GhiyJj5s-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202302150540.GhiyJj5s-lkp@intel.com/
smatch warnings:
drivers/regulator/pca9450-regulator.c:866 pca9450_i2c_probe() error: uninitialized symbol 'ldo5'.
vim +/ldo5 +866 drivers/regulator/pca9450-regulator.c
0935ff5f1f0a44 Robin Gong 2020-07-04 721
ed56fa6e804cb1 Uwe Kleine-König 2022-11-18 722 static int pca9450_i2c_probe(struct i2c_client *i2c)
0935ff5f1f0a44 Robin Gong 2020-07-04 723 {
0935ff5f1f0a44 Robin Gong 2020-07-04 724 enum pca9450_chip_type type = (unsigned int)(uintptr_t)
0935ff5f1f0a44 Robin Gong 2020-07-04 725 of_device_get_match_data(&i2c->dev);
0935ff5f1f0a44 Robin Gong 2020-07-04 726 const struct pca9450_regulator_desc *regulator_desc;
0935ff5f1f0a44 Robin Gong 2020-07-04 727 struct regulator_config config = { };
f722e874661add Frieder Schrempf 2023-02-13 728 struct regulator_dev *ldo5;
0935ff5f1f0a44 Robin Gong 2020-07-04 729 struct pca9450 *pca9450;
0935ff5f1f0a44 Robin Gong 2020-07-04 730 unsigned int device_id, i;
2364a64d0673f5 Rickard x Andersson 2022-04-29 731 unsigned int reset_ctrl;
0935ff5f1f0a44 Robin Gong 2020-07-04 732 int ret;
0935ff5f1f0a44 Robin Gong 2020-07-04 733
0935ff5f1f0a44 Robin Gong 2020-07-04 734 if (!i2c->irq) {
0935ff5f1f0a44 Robin Gong 2020-07-04 735 dev_err(&i2c->dev, "No IRQ configured?\n");
0935ff5f1f0a44 Robin Gong 2020-07-04 736 return -EINVAL;
0935ff5f1f0a44 Robin Gong 2020-07-04 737 }
0935ff5f1f0a44 Robin Gong 2020-07-04 738
0935ff5f1f0a44 Robin Gong 2020-07-04 739 pca9450 = devm_kzalloc(&i2c->dev, sizeof(struct pca9450), GFP_KERNEL);
0935ff5f1f0a44 Robin Gong 2020-07-04 740 if (!pca9450)
0935ff5f1f0a44 Robin Gong 2020-07-04 741 return -ENOMEM;
0935ff5f1f0a44 Robin Gong 2020-07-04 742
0935ff5f1f0a44 Robin Gong 2020-07-04 743 switch (type) {
0935ff5f1f0a44 Robin Gong 2020-07-04 744 case PCA9450_TYPE_PCA9450A:
0935ff5f1f0a44 Robin Gong 2020-07-04 745 regulator_desc = pca9450a_regulators;
0935ff5f1f0a44 Robin Gong 2020-07-04 746 pca9450->rcnt = ARRAY_SIZE(pca9450a_regulators);
0935ff5f1f0a44 Robin Gong 2020-07-04 747 break;
0935ff5f1f0a44 Robin Gong 2020-07-04 748 case PCA9450_TYPE_PCA9450BC:
0935ff5f1f0a44 Robin Gong 2020-07-04 749 regulator_desc = pca9450bc_regulators;
0935ff5f1f0a44 Robin Gong 2020-07-04 750 pca9450->rcnt = ARRAY_SIZE(pca9450bc_regulators);
0935ff5f1f0a44 Robin Gong 2020-07-04 751 break;
0935ff5f1f0a44 Robin Gong 2020-07-04 752 default:
0935ff5f1f0a44 Robin Gong 2020-07-04 753 dev_err(&i2c->dev, "Unknown device type");
0935ff5f1f0a44 Robin Gong 2020-07-04 754 return -EINVAL;
0935ff5f1f0a44 Robin Gong 2020-07-04 755 }
0935ff5f1f0a44 Robin Gong 2020-07-04 756
0935ff5f1f0a44 Robin Gong 2020-07-04 757 pca9450->irq = i2c->irq;
0935ff5f1f0a44 Robin Gong 2020-07-04 758 pca9450->type = type;
0935ff5f1f0a44 Robin Gong 2020-07-04 759 pca9450->dev = &i2c->dev;
0935ff5f1f0a44 Robin Gong 2020-07-04 760
0935ff5f1f0a44 Robin Gong 2020-07-04 761 dev_set_drvdata(&i2c->dev, pca9450);
0935ff5f1f0a44 Robin Gong 2020-07-04 762
0935ff5f1f0a44 Robin Gong 2020-07-04 763 pca9450->regmap = devm_regmap_init_i2c(i2c,
0935ff5f1f0a44 Robin Gong 2020-07-04 764 &pca9450_regmap_config);
0935ff5f1f0a44 Robin Gong 2020-07-04 765 if (IS_ERR(pca9450->regmap)) {
0935ff5f1f0a44 Robin Gong 2020-07-04 766 dev_err(&i2c->dev, "regmap initialization failed\n");
0935ff5f1f0a44 Robin Gong 2020-07-04 767 return PTR_ERR(pca9450->regmap);
0935ff5f1f0a44 Robin Gong 2020-07-04 768 }
0935ff5f1f0a44 Robin Gong 2020-07-04 769
0935ff5f1f0a44 Robin Gong 2020-07-04 770 ret = regmap_read(pca9450->regmap, PCA9450_REG_DEV_ID, &device_id);
0935ff5f1f0a44 Robin Gong 2020-07-04 771 if (ret) {
0935ff5f1f0a44 Robin Gong 2020-07-04 772 dev_err(&i2c->dev, "Read device id error\n");
0935ff5f1f0a44 Robin Gong 2020-07-04 773 return ret;
0935ff5f1f0a44 Robin Gong 2020-07-04 774 }
0935ff5f1f0a44 Robin Gong 2020-07-04 775
0935ff5f1f0a44 Robin Gong 2020-07-04 776 /* Check your board and dts for match the right pmic */
0935ff5f1f0a44 Robin Gong 2020-07-04 777 if (((device_id >> 4) != 0x1 && type == PCA9450_TYPE_PCA9450A) ||
0935ff5f1f0a44 Robin Gong 2020-07-04 778 ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC)) {
0935ff5f1f0a44 Robin Gong 2020-07-04 779 dev_err(&i2c->dev, "Device id(%x) mismatched\n",
0935ff5f1f0a44 Robin Gong 2020-07-04 780 device_id >> 4);
0935ff5f1f0a44 Robin Gong 2020-07-04 781 return -EINVAL;
0935ff5f1f0a44 Robin Gong 2020-07-04 782 }
0935ff5f1f0a44 Robin Gong 2020-07-04 783
0935ff5f1f0a44 Robin Gong 2020-07-04 784 for (i = 0; i < pca9450->rcnt; i++) {
0935ff5f1f0a44 Robin Gong 2020-07-04 785 const struct regulator_desc *desc;
0935ff5f1f0a44 Robin Gong 2020-07-04 786 struct regulator_dev *rdev;
0935ff5f1f0a44 Robin Gong 2020-07-04 787 const struct pca9450_regulator_desc *r;
0935ff5f1f0a44 Robin Gong 2020-07-04 788
0935ff5f1f0a44 Robin Gong 2020-07-04 789 r = ®ulator_desc[i];
0935ff5f1f0a44 Robin Gong 2020-07-04 790 desc = &r->desc;
0935ff5f1f0a44 Robin Gong 2020-07-04 791
0935ff5f1f0a44 Robin Gong 2020-07-04 792 config.regmap = pca9450->regmap;
0935ff5f1f0a44 Robin Gong 2020-07-04 793 config.dev = pca9450->dev;
f722e874661add Frieder Schrempf 2023-02-13 794 config.driver_data = pca9450;
0935ff5f1f0a44 Robin Gong 2020-07-04 795
0935ff5f1f0a44 Robin Gong 2020-07-04 796 rdev = devm_regulator_register(pca9450->dev, desc, &config);
0935ff5f1f0a44 Robin Gong 2020-07-04 797 if (IS_ERR(rdev)) {
0935ff5f1f0a44 Robin Gong 2020-07-04 798 ret = PTR_ERR(rdev);
0935ff5f1f0a44 Robin Gong 2020-07-04 799 dev_err(pca9450->dev,
0935ff5f1f0a44 Robin Gong 2020-07-04 800 "Failed to register regulator(%s): %d\n",
0935ff5f1f0a44 Robin Gong 2020-07-04 801 desc->name, ret);
0935ff5f1f0a44 Robin Gong 2020-07-04 802 return ret;
0935ff5f1f0a44 Robin Gong 2020-07-04 803 }
f722e874661add Frieder Schrempf 2023-02-13 804
f722e874661add Frieder Schrempf 2023-02-13 805 if (!strcmp(desc->name, "ldo5"))
f722e874661add Frieder Schrempf 2023-02-13 806 ldo5 = rdev;
0935ff5f1f0a44 Robin Gong 2020-07-04 807 }
0935ff5f1f0a44 Robin Gong 2020-07-04 808
0935ff5f1f0a44 Robin Gong 2020-07-04 809 ret = devm_request_threaded_irq(pca9450->dev, pca9450->irq, NULL,
0935ff5f1f0a44 Robin Gong 2020-07-04 810 pca9450_irq_handler,
0935ff5f1f0a44 Robin Gong 2020-07-04 811 (IRQF_TRIGGER_FALLING | IRQF_ONESHOT),
0935ff5f1f0a44 Robin Gong 2020-07-04 812 "pca9450-irq", pca9450);
0935ff5f1f0a44 Robin Gong 2020-07-04 813 if (ret != 0) {
0935ff5f1f0a44 Robin Gong 2020-07-04 814 dev_err(pca9450->dev, "Failed to request IRQ: %d\n",
0935ff5f1f0a44 Robin Gong 2020-07-04 815 pca9450->irq);
0935ff5f1f0a44 Robin Gong 2020-07-04 816 return ret;
0935ff5f1f0a44 Robin Gong 2020-07-04 817 }
0935ff5f1f0a44 Robin Gong 2020-07-04 818 /* Unmask all interrupt except PWRON/WDOG/RSVD */
0935ff5f1f0a44 Robin Gong 2020-07-04 819 ret = regmap_update_bits(pca9450->regmap, PCA9450_REG_INT1_MSK,
0935ff5f1f0a44 Robin Gong 2020-07-04 820 IRQ_VR_FLT1 | IRQ_VR_FLT2 | IRQ_LOWVSYS |
0935ff5f1f0a44 Robin Gong 2020-07-04 821 IRQ_THERM_105 | IRQ_THERM_125,
0935ff5f1f0a44 Robin Gong 2020-07-04 822 IRQ_PWRON | IRQ_WDOGB | IRQ_RSVD);
0935ff5f1f0a44 Robin Gong 2020-07-04 823 if (ret) {
0935ff5f1f0a44 Robin Gong 2020-07-04 824 dev_err(&i2c->dev, "Unmask irq error\n");
0935ff5f1f0a44 Robin Gong 2020-07-04 825 return ret;
0935ff5f1f0a44 Robin Gong 2020-07-04 826 }
0935ff5f1f0a44 Robin Gong 2020-07-04 827
98b94b6e38ca0c Frieder Schrempf 2021-02-22 828 /* Clear PRESET_EN bit in BUCK123_DVS to use DVS registers */
98b94b6e38ca0c Frieder Schrempf 2021-02-22 829 ret = regmap_clear_bits(pca9450->regmap, PCA9450_REG_BUCK123_DVS,
98b94b6e38ca0c Frieder Schrempf 2021-02-22 830 BUCK123_PRESET_EN);
98b94b6e38ca0c Frieder Schrempf 2021-02-22 831 if (ret) {
98b94b6e38ca0c Frieder Schrempf 2021-02-22 832 dev_err(&i2c->dev, "Failed to clear PRESET_EN bit: %d\n", ret);
98b94b6e38ca0c Frieder Schrempf 2021-02-22 833 return ret;
98b94b6e38ca0c Frieder Schrempf 2021-02-22 834 }
98b94b6e38ca0c Frieder Schrempf 2021-02-22 835
2364a64d0673f5 Rickard x Andersson 2022-04-29 836 if (of_property_read_bool(i2c->dev.of_node, "nxp,wdog_b-warm-reset"))
2364a64d0673f5 Rickard x Andersson 2022-04-29 837 reset_ctrl = WDOG_B_CFG_WARM;
2364a64d0673f5 Rickard x Andersson 2022-04-29 838 else
2364a64d0673f5 Rickard x Andersson 2022-04-29 839 reset_ctrl = WDOG_B_CFG_COLD_LDO12;
2364a64d0673f5 Rickard x Andersson 2022-04-29 840
f7684f5a048feb Frieder Schrempf 2021-02-11 841 /* Set reset behavior on assertion of WDOG_B signal */
f7684f5a048feb Frieder Schrempf 2021-02-11 842 ret = regmap_update_bits(pca9450->regmap, PCA9450_REG_RESET_CTRL,
2364a64d0673f5 Rickard x Andersson 2022-04-29 843 WDOG_B_CFG_MASK, reset_ctrl);
f7684f5a048feb Frieder Schrempf 2021-02-11 844 if (ret) {
f7684f5a048feb Frieder Schrempf 2021-02-11 845 dev_err(&i2c->dev, "Failed to set WDOG_B reset behavior\n");
f7684f5a048feb Frieder Schrempf 2021-02-11 846 return ret;
f7684f5a048feb Frieder Schrempf 2021-02-11 847 }
f7684f5a048feb Frieder Schrempf 2021-02-11 848
62139f52b7e588 Per-Daniel Olsson 2022-04-29 849 if (of_property_read_bool(i2c->dev.of_node, "nxp,i2c-lt-enable")) {
62139f52b7e588 Per-Daniel Olsson 2022-04-29 850 /* Enable I2C Level Translator */
62139f52b7e588 Per-Daniel Olsson 2022-04-29 851 ret = regmap_update_bits(pca9450->regmap, PCA9450_REG_CONFIG2,
62139f52b7e588 Per-Daniel Olsson 2022-04-29 852 I2C_LT_MASK, I2C_LT_ON_STANDBY_RUN);
62139f52b7e588 Per-Daniel Olsson 2022-04-29 853 if (ret) {
62139f52b7e588 Per-Daniel Olsson 2022-04-29 854 dev_err(&i2c->dev,
62139f52b7e588 Per-Daniel Olsson 2022-04-29 855 "Failed to enable I2C level translator\n");
62139f52b7e588 Per-Daniel Olsson 2022-04-29 856 return ret;
62139f52b7e588 Per-Daniel Olsson 2022-04-29 857 }
62139f52b7e588 Per-Daniel Olsson 2022-04-29 858 }
62139f52b7e588 Per-Daniel Olsson 2022-04-29 859
f722e874661add Frieder Schrempf 2023-02-13 860 /*
f722e874661add Frieder Schrempf 2023-02-13 861 * For LDO5 we need to be able to check the status of the SD_VSEL input in
f722e874661add Frieder Schrempf 2023-02-13 862 * order to know which control register is used. Most boards connect SD_VSEL
f722e874661add Frieder Schrempf 2023-02-13 863 * to the VSELECT signal, so we can use the GPIO that is internally routed
f722e874661add Frieder Schrempf 2023-02-13 864 * to this signal (if SION bit is set in IOMUX).
f722e874661add Frieder Schrempf 2023-02-13 865 */
f722e874661add Frieder Schrempf 2023-02-13 @866 pca9450->sd_vsel_gpio = gpiod_get_optional(&ldo5->dev, "sd-vsel", GPIOD_IN);
f722e874661add Frieder Schrempf 2023-02-13 867 if (IS_ERR(pca9450->sd_vsel_gpio)) {
f722e874661add Frieder Schrempf 2023-02-13 868 dev_err(&i2c->dev, "Failed to get SD_VSEL GPIO\n");
f722e874661add Frieder Schrempf 2023-02-13 869 return ret;
f722e874661add Frieder Schrempf 2023-02-13 870 }
f722e874661add Frieder Schrempf 2023-02-13 871
0935ff5f1f0a44 Robin Gong 2020-07-04 872 dev_info(&i2c->dev, "%s probed.\n",
0935ff5f1f0a44 Robin Gong 2020-07-04 873 type == PCA9450_TYPE_PCA9450A ? "pca9450a" : "pca9450bc");
0935ff5f1f0a44 Robin Gong 2020-07-04 874
0935ff5f1f0a44 Robin Gong 2020-07-04 875 return 0;
0935ff5f1f0a44 Robin Gong 2020-07-04 876 }
0935ff5f1f0a44 Robin Gong 2020-07-04 877
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2023-02-16 14:32 UTC | newest]
Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-13 15:58 [PATCH 0/6] Use correct LDO5 control registers for PCA9450 Frieder Schrempf
2023-02-13 15:58 ` Frieder Schrempf
2023-02-13 15:58 ` [PATCH 1/6] dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios Frieder Schrempf
2023-02-15 20:02 ` Rob Herring
2023-02-16 1:27 ` Marek Vasut
2023-02-16 2:30 ` Rob Herring
2023-02-16 10:15 ` Frieder Schrempf
2023-02-13 15:58 ` [PATCH 2/6] regulator: pca9450: Fix enable register for LDO5 Frieder Schrempf
2023-02-13 16:12 ` Marek Vasut
2023-02-13 15:58 ` [PATCH 3/6] Revert "regulator: pca9450: Add SD_VSEL GPIO for LDO5" Frieder Schrempf
2023-02-13 16:16 ` Marek Vasut
2023-02-13 16:23 ` Frieder Schrempf
2023-02-13 18:10 ` Marek Vasut
2023-02-13 15:58 ` [PATCH 4/6] regulator: Add operation to let drivers select vsel register Frieder Schrempf
2023-02-14 1:40 ` kernel test robot
2023-02-14 9:19 ` kernel test robot
2023-02-14 21:06 ` Mark Brown
2023-02-16 9:05 ` Frieder Schrempf
2023-02-16 14:31 ` Mark Brown
2023-02-13 15:58 ` [PATCH 5/6] regulator: pca9450: Fix control register for LDO5 Frieder Schrempf
2023-02-13 15:58 ` [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal Frieder Schrempf
2023-02-13 15:58 ` Frieder Schrempf
2023-02-13 16:08 ` Marek Vasut
2023-02-13 16:08 ` Marek Vasut
2023-02-13 16:15 ` Frieder Schrempf
2023-02-13 16:15 ` Frieder Schrempf
2023-02-13 16:15 ` Marco Felsch
2023-02-13 16:15 ` Marco Felsch
2023-02-13 16:18 ` Frieder Schrempf
2023-02-13 16:18 ` Frieder Schrempf
2023-02-13 21:02 ` Fabio Estevam
2023-02-13 21:02 ` Fabio Estevam
2023-02-13 18:12 ` Marek Vasut
2023-02-13 18:12 ` Marek Vasut
2023-02-13 19:56 ` Marco Felsch
2023-02-13 19:56 ` Marco Felsch
2023-02-13 20:59 ` Marek Vasut
2023-02-13 20:59 ` Marek Vasut
2023-02-14 8:10 ` Marco Felsch
2023-02-14 8:10 ` Marco Felsch
2023-02-14 8:26 ` Frieder Schrempf
2023-02-14 8:26 ` Frieder Schrempf
2023-02-14 11:46 ` Marco Felsch
2023-02-14 11:46 ` Marco Felsch
2023-02-14 21:52 [PATCH 5/6] regulator: pca9450: Fix control register for LDO5 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.