[PATCH 0/5] Add st-flashss vsense regulator driver

[PATCH 0/5] Add st-flashss vsense regulator driver

[Peter Griffin]

Hi Mark,

This series adds a small regulator driver to manage the voltage regulator
inside the ST flash subsystem found on stih407 based silicon, and it is
used for configuring the eMMC, NAND and SPI voltages (although only
currently utilised for eMMC).

regards,

Peter.

Peter Griffin (5):
  regulator: st-flashss: Add DT binding documentation for flashss
    regulator.
  regulator: st-flashss: Add a regulator driver for flashss vsense.
  MAINTAINERS: Add st-flashss driver to STi section.
  ARM: multi_v7_defconfig: Enable flashss regulator driver.
  ARM: STi: DT: STiH407: Add the flashss voltage regulator DT node.

 .../devicetree/bindings/regulator/st-flashss.txt   |  43 ++++
 MAINTAINERS                                        |   1 +
 arch/arm/boot/dts/stih407-family.dtsi              |  12 +
 arch/arm/boot/dts/stihxxx-b2120.dtsi               |   4 +
 arch/arm/configs/multi_v7_defconfig                |   1 +
 drivers/regulator/Kconfig                          |   7 +
 drivers/regulator/Makefile                         |   2 +-
 drivers/regulator/st-flashss.c                     | 274 +++++++++++++++++++++
 8 files changed, 343 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/st-flashss.txt
 create mode 100644 drivers/regulator/st-flashss.c

-- 
1.9.1

[PATCH 0/5] Add st-flashss vsense regulator driver

[Peter Griffin]

Hi Mark,

This series adds a small regulator driver to manage the voltage regulator
inside the ST flash subsystem found on stih407 based silicon, and it is
used for configuring the eMMC, NAND and SPI voltages (although only
currently utilised for eMMC).

regards,

Peter.

Peter Griffin (5):
  regulator: st-flashss: Add DT binding documentation for flashss
    regulator.
  regulator: st-flashss: Add a regulator driver for flashss vsense.
  MAINTAINERS: Add st-flashss driver to STi section.
  ARM: multi_v7_defconfig: Enable flashss regulator driver.
  ARM: STi: DT: STiH407: Add the flashss voltage regulator DT node.

 .../devicetree/bindings/regulator/st-flashss.txt   |  43 ++++
 MAINTAINERS                                        |   1 +
 arch/arm/boot/dts/stih407-family.dtsi              |  12 +
 arch/arm/boot/dts/stihxxx-b2120.dtsi               |   4 +
 arch/arm/configs/multi_v7_defconfig                |   1 +
 drivers/regulator/Kconfig                          |   7 +
 drivers/regulator/Makefile                         |   2 +-
 drivers/regulator/st-flashss.c                     | 274 +++++++++++++++++++++
 8 files changed, 343 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/st-flashss.txt
 create mode 100644 drivers/regulator/st-flashss.c

-- 
1.9.1

[PATCH 1/5] regulator: st-flashss: Add DT binding documentation for flashss regulator.

[Peter Griffin]

This patch adds the DT doc for the flashss regulator driver which is
found in stih407 based silicon.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 .../devicetree/bindings/regulator/st-flashss.txt   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/st-flashss.txt

diff --git a/Documentation/devicetree/bindings/regulator/st-flashss.txt b/Documentation/devicetree/bindings/regulator/st-flashss.txt
new file mode 100644
index 0000000..04324d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/st-flashss.txt
@@ -0,0 +1,43 @@
+ST Voltage regulator for FlashSS vsense
+
+This documents the voltage regulator driver which manages the vsense inside the ST flash
+sub-system that is used for configuring MMC, NAND, SPI voltages.
+
+Required properties:
+- compatible:		Can be "st,vqmmc", "st,vnand" or "st,vspi" because it is shared
+			among these devices inside the ST FlashSS.
+- regulator-name:	Regulator name.
+- reg:			The vsense top config base address in the flashSS hardware.
+
+Any property defined as part of the core regulator binding, in
+Documentation/devicetree/bindings/regulator/regulator.txt can also be used.
+
+Example:
+
+vqmmc_reg: voltage-regulator {
+	compatible = "st,vqmmc";
+	regulator-name = "vqmmc0";
+	/* Control register for the VSENSE Module */
+	reg = <0x9061004 0x4>;
+	status = "disabled";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-always-on;
+};
+
+mmc0: sdhci@09060000 {
+	compatible = "st,sdhci-stih407", "st,sdhci";
+	status = "disabled";
+	reg = <0x09060000 0x7ff>, <0x9061008 0x20>;
+	reg-names = "mmc", "top-mmc-delay";
+	interrupts = <GIC_SPI 92 IRQ_TYPE_NONE>;
+	interrupt-names = "mmcirq";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_mmc0>;
+	clock-names = "mmc";
+	clocks = <&clk_s_c0_flexgen CLK_MMC_0>;
+	bus-width = <8>;
+	non-removable;
+	vqmmc-supply = <&vqmmc_reg>;
+};
+
-- 
1.9.1

[PATCH 1/5] regulator: st-flashss: Add DT binding documentation for flashss regulator.

[Peter Griffin]

This patch adds the DT doc for the flashss regulator driver which is
found in stih407 based silicon.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 .../devicetree/bindings/regulator/st-flashss.txt   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/st-flashss.txt

diff --git a/Documentation/devicetree/bindings/regulator/st-flashss.txt b/Documentation/devicetree/bindings/regulator/st-flashss.txt
new file mode 100644
index 0000000..04324d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/st-flashss.txt
@@ -0,0 +1,43 @@
+ST Voltage regulator for FlashSS vsense
+
+This documents the voltage regulator driver which manages the vsense inside the ST flash
+sub-system that is used for configuring MMC, NAND, SPI voltages.
+
+Required properties:
+- compatible:		Can be "st,vqmmc", "st,vnand" or "st,vspi" because it is shared
+			among these devices inside the ST FlashSS.
+- regulator-name:	Regulator name.
+- reg:			The vsense top config base address in the flashSS hardware.
+
+Any property defined as part of the core regulator binding, in
+Documentation/devicetree/bindings/regulator/regulator.txt can also be used.
+
+Example:
+
+vqmmc_reg: voltage-regulator {
+	compatible = "st,vqmmc";
+	regulator-name = "vqmmc0";
+	/* Control register for the VSENSE Module */
+	reg = <0x9061004 0x4>;
+	status = "disabled";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-always-on;
+};
+
+mmc0: sdhci@09060000 {
+	compatible = "st,sdhci-stih407", "st,sdhci";
+	status = "disabled";
+	reg = <0x09060000 0x7ff>, <0x9061008 0x20>;
+	reg-names = "mmc", "top-mmc-delay";
+	interrupts = <GIC_SPI 92 IRQ_TYPE_NONE>;
+	interrupt-names = "mmcirq";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_mmc0>;
+	clock-names = "mmc";
+	clocks = <&clk_s_c0_flexgen CLK_MMC_0>;
+	bus-width = <8>;
+	non-removable;
+	vqmmc-supply = <&vqmmc_reg>;
+};
+
-- 
1.9.1

[PATCH 1/5] regulator: st-flashss: Add DT binding documentation for flashss regulator.

[Peter Griffin]

This patch adds the DT doc for the flashss regulator driver which is
found in stih407 based silicon.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 .../devicetree/bindings/regulator/st-flashss.txt   | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/st-flashss.txt

diff --git a/Documentation/devicetree/bindings/regulator/st-flashss.txt b/Documentation/devicetree/bindings/regulator/st-flashss.txt
new file mode 100644
index 0000000..04324d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/st-flashss.txt
@@ -0,0 +1,43 @@
+ST Voltage regulator for FlashSS vsense
+
+This documents the voltage regulator driver which manages the vsense inside the ST flash
+sub-system that is used for configuring MMC, NAND, SPI voltages.
+
+Required properties:
+- compatible:		Can be "st,vqmmc", "st,vnand" or "st,vspi" because it is shared
+			among these devices inside the ST FlashSS.
+- regulator-name:	Regulator name.
+- reg:			The vsense top config base address in the flashSS hardware.
+
+Any property defined as part of the core regulator binding, in
+Documentation/devicetree/bindings/regulator/regulator.txt can also be used.
+
+Example:
+
+vqmmc_reg: voltage-regulator {
+	compatible = "st,vqmmc";
+	regulator-name = "vqmmc0";
+	/* Control register for the VSENSE Module */
+	reg = <0x9061004 0x4>;
+	status = "disabled";
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-always-on;
+};
+
+mmc0: sdhci at 09060000 {
+	compatible = "st,sdhci-stih407", "st,sdhci";
+	status = "disabled";
+	reg = <0x09060000 0x7ff>, <0x9061008 0x20>;
+	reg-names = "mmc", "top-mmc-delay";
+	interrupts = <GIC_SPI 92 IRQ_TYPE_NONE>;
+	interrupt-names = "mmcirq";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_mmc0>;
+	clock-names = "mmc";
+	clocks = <&clk_s_c0_flexgen CLK_MMC_0>;
+	bus-width = <8>;
+	non-removable;
+	vqmmc-supply = <&vqmmc_reg>;
+};
+
-- 
1.9.1

[PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Peter Griffin]

This patch adds a small regulator driver to manage the voltage regulator
inside the ST flash subsystem found on stih407 based silicon, and is
used for configuring the eMMC, NAND and SPI voltages.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/regulator/Kconfig      |   7 ++
 drivers/regulator/Makefile     |   2 +-
 drivers/regulator/st-flashss.c | 274 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 282 insertions(+), 1 deletion(-)
 create mode 100644 drivers/regulator/st-flashss.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c77dc08..ddf2bcd 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -843,5 +843,12 @@ config REGULATOR_WM8994
 	  This driver provides support for the voltage regulators on the
 	  WM8994 CODEC.
 
+config REGULATOR_ST_FLASHSS
+	tristate "STMicroelectronics FlashSS regulator"
+	depends on ARCH_STI && OF
+	help
+	  This driver provides support for the voltage regulators available
+	  inside the FlashSS of STiH407/STiH410 SoC's.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 61bfbb9..10be29b 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -108,6 +108,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
 obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
 obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
 obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
-
+obj-$(CONFIG_REGULATOR_ST_FLASHSS) += st-flashss.o
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/st-flashss.c b/drivers/regulator/st-flashss.c
new file mode 100644
index 0000000..9c0e011
--- /dev/null
+++ b/drivers/regulator/st-flashss.c
@@ -0,0 +1,274 @@
+/*
+ * ST regulator driver for flashSS vsense
+ *
+ * This is a small driver to manage the voltage regulator inside the ST flash
+ * sub-system that is used for configuring MMC, NAND, SPI voltages.
+ *
+ * Copyright(C) 2015 STMicroelectronics Ltd
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under  the terms of the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+
+/* FlashSS voltage VSENSE TOP CONFIG register defines */
+#define TOP_VSENSE_CONFIG_REG_PSW_EMMC		BIT(0)
+#define TOP_VSENSE_CONFIG_ENB_REG_PSW_EMMC	BIT(1)
+#define TOP_VSENSE_CONFIG_REG_PSW_NAND		BIT(8)
+#define TOP_VSENSE_CONFIG_ENB_REG_PSW_NAND	BIT(9)
+#define TOP_VSENSE_CONFIG_REG_PSW_SPI		BIT(16)
+#define TOP_VSENSE_CONFIG_ENB_REG_PSW_SPI	BIT(17)
+#define TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC	BIT(24)
+#define TOP_VSENSE_CONFIG_LATCHED_PSW_NAND	BIT(25)
+#define TOP_VSENSE_CONFIG_LATCHED_PSW_SPI	BIT(26)
+
+struct st_vsense {
+	char *name;
+	struct device *dev;
+	u8 n_voltages;			/* number of supported voltages */
+	void __iomem *ioaddr;		/* TOP config base address */
+	unsigned int en_psw_mask;	/* Mask/enable vdd for each device */
+	unsigned int psw_mask;		/* Power sel mask for VDD */
+	unsigned int latched_mask;	/* Latched mask for VDD */
+};
+
+static const unsigned int st_type_voltages[] = {
+	1800000,
+	3300000,
+};
+
+const struct st_vsense st_vsense_data[] = {
+	{
+		.en_psw_mask = TOP_VSENSE_CONFIG_ENB_REG_PSW_EMMC,
+		.psw_mask = TOP_VSENSE_CONFIG_REG_PSW_EMMC,
+		.latched_mask = TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC,
+	}, {
+		.en_psw_mask = TOP_VSENSE_CONFIG_ENB_REG_PSW_NAND,
+		.psw_mask = TOP_VSENSE_CONFIG_REG_PSW_NAND,
+		.latched_mask = TOP_VSENSE_CONFIG_LATCHED_PSW_NAND,
+	}, {
+		.en_psw_mask = TOP_VSENSE_CONFIG_ENB_REG_PSW_SPI,
+		.psw_mask = TOP_VSENSE_CONFIG_REG_PSW_SPI,
+		.latched_mask = TOP_VSENSE_CONFIG_LATCHED_PSW_SPI,
+	},
+};
+
+/* Get the value of Sensed-PSW of eMMC/NAND/SPI Pads */
+static int st_get_voltage_sel(struct regulator_dev *dev)
+{
+	struct st_vsense *vsense = rdev_get_drvdata(dev);
+	void __iomem *ioaddr = vsense->ioaddr;
+	int sel = 0;
+	u32 value = readl_relaxed(ioaddr);
+
+	if (value & vsense->latched_mask)
+		sel = 1;
+
+	dev_dbg(vsense->dev, "%s, selection %d (0x%08x)\n", vsense->name, sel,
+		readl_relaxed(ioaddr));
+
+	return sel;
+}
+
+static int st_set_voltage_sel(struct regulator_dev *dev, unsigned int selector)
+{
+	struct st_vsense *vsense = rdev_get_drvdata(dev);
+	void __iomem *ioaddr = vsense->ioaddr;
+	unsigned int value = readl_relaxed(ioaddr);
+	unsigned int voltage;
+
+	voltage = st_type_voltages[selector];
+
+	value |= vsense->en_psw_mask;
+	if (voltage == 3300000)
+		value |= vsense->psw_mask;
+	else
+		value &= ~vsense->psw_mask;
+
+	writel_relaxed(value, ioaddr);
+
+	dev_dbg(vsense->dev, "%s, required voltage %d (vsense_conf 0x%08x)\n",
+		vsense->name, voltage,
+		readl_relaxed(ioaddr));
+
+	return 0;
+}
+
+static struct regulator_ops st_ops = {
+	.get_voltage_sel = st_get_voltage_sel,
+	.set_voltage_sel = st_set_voltage_sel,
+	.list_voltage = regulator_list_voltage_table,
+};
+
+static const struct of_device_id of_st_match_tbl[] = {
+	{.compatible = "st,vqmmc", .data = &st_vsense_data[0]},
+	{.compatible = "st,vnand", .data = &st_vsense_data[1]},
+	{.compatible = "st,vspi", .data = &st_vsense_data[2]},
+	{ /* end */ }
+};
+
+static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
+{
+	void __iomem *ioaddr = vsense->ioaddr;
+	u32 value = readl_relaxed(ioaddr);
+
+	dev_dbg(vsense->dev, "Initial start-up value: (0x%08x)\n", value);
+
+	/* Sanitize voltage values forcing what is provided from start-up */
+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC)
+		value |= TOP_VSENSE_CONFIG_REG_PSW_EMMC;
+	else
+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;
+
+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND)
+		value |= TOP_VSENSE_CONFIG_REG_PSW_NAND;
+	else
+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;
+
+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI)
+		value |= TOP_VSENSE_CONFIG_REG_PSW_SPI;
+	else
+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;
+
+	writel_relaxed(value, ioaddr);
+
+	dev_dbg(vsense->dev, "Sanitized value: (0x%08x)\n", value);
+}
+
+static int st_vsense_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct st_vsense *vsense;
+	const struct of_device_id *device;
+	struct resource *res;
+	struct regulator_desc *rdesc;
+	struct regulator_dev *rdev;
+	struct regulator_config config = { };
+
+	vsense = devm_kzalloc(dev, sizeof(*vsense), GFP_KERNEL);
+	if (!vsense)
+		return -ENOMEM;
+
+	vsense->dev = dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	vsense->ioaddr = devm_ioremap_resource(dev, res);
+	if (IS_ERR(vsense->ioaddr))
+		return PTR_ERR(vsense->ioaddr);
+
+	rdesc = devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL);
+	if (!rdesc)
+		return -ENOMEM;
+
+	device = of_match_device(of_st_match_tbl, &pdev->dev);
+	if (!device)
+		return -ENODEV;
+
+	if (device->data) {
+		const struct st_vsense *data = device->data;
+
+		vsense->en_psw_mask = data->en_psw_mask;
+		vsense->psw_mask = data->psw_mask;
+		vsense->latched_mask = data->latched_mask;
+	} else
+		return -ENODEV;
+
+	if (of_property_read_string(np, "regulator-name",
+				    (const char **)&vsense->name))
+		return -EINVAL;
+
+	memset(rdesc, 0, sizeof(*rdesc));
+	rdesc->name = vsense->name;
+	rdesc->ops = &st_ops;
+	rdesc->type = REGULATOR_VOLTAGE;
+	rdesc->owner = THIS_MODULE;
+	rdesc->n_voltages = ARRAY_SIZE(st_type_voltages);
+	rdesc->volt_table = st_type_voltages;
+	config.dev = &pdev->dev;
+	config.driver_data = vsense;
+	config.of_node = np;
+
+	config.init_data = of_get_regulator_init_data(&pdev->dev, np, rdesc);
+	if (!config.init_data) {
+		dev_err(dev, "Failed to parse regulator init data\n");
+		return -ENOMEM;
+	}
+
+	/* register regulator */
+	rdev = regulator_register(rdesc, &config);
+	if (IS_ERR(rdev)) {
+		dev_err(dev, "failed to register %s\n", rdesc->name);
+		return PTR_ERR(rdev);
+	}
+
+	platform_set_drvdata(pdev, rdev);
+
+	dev_info(dev, "%s  vsense voltage regulator registered\n", rdesc->name);
+	st_get_satinize_powerup_voltage(vsense);
+
+	return 0;
+}
+
+static int st_vsense_remove(struct platform_device *pdev)
+{
+	struct regulator_dev *rdev = platform_get_drvdata(pdev);
+
+	regulator_unregister(rdev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int st_vsense_resume(struct device *dev)
+{
+	struct regulator_dev *rdev = dev_get_drvdata(dev);
+	struct st_vsense *vsense = rdev_get_drvdata(rdev);
+
+	st_get_satinize_powerup_voltage(vsense);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops st_vsense_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(NULL, st_vsense_resume)
+};
+
+static struct platform_driver st_vsense_driver = {
+	.driver = {
+		   .name = "st-vsense",
+		   .of_match_table = of_st_match_tbl,
+		   .pm	= &st_vsense_pm_ops,
+		   },
+	.probe = st_vsense_probe,
+	.remove = st_vsense_remove,
+};
+
+static int __init st_vsense_init(void)
+{
+	return platform_driver_register(&st_vsense_driver);
+}
+
+subsys_initcall(st_vsense_init);
+
+static void __exit st_vsense_cleanup(void)
+{
+	platform_driver_unregister(&st_vsense_driver);
+}
+
+module_exit(st_vsense_cleanup);
+
+MODULE_AUTHOR("Giuseppe Cavallaro <peppe.cavallaro@st.com>");
+MODULE_DESCRIPTION("ST voltage regulator driver for vsense flashSS");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Peter Griffin]

This patch adds a small regulator driver to manage the voltage regulator
inside the ST flash subsystem found on stih407 based silicon, and is
used for configuring the eMMC, NAND and SPI voltages.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/regulator/Kconfig      |   7 ++
 drivers/regulator/Makefile     |   2 +-
 drivers/regulator/st-flashss.c | 274 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 282 insertions(+), 1 deletion(-)
 create mode 100644 drivers/regulator/st-flashss.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c77dc08..ddf2bcd 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -843,5 +843,12 @@ config REGULATOR_WM8994
 	  This driver provides support for the voltage regulators on the
 	  WM8994 CODEC.
 
+config REGULATOR_ST_FLASHSS
+	tristate "STMicroelectronics FlashSS regulator"
+	depends on ARCH_STI && OF
+	help
+	  This driver provides support for the voltage regulators available
+	  inside the FlashSS of STiH407/STiH410 SoC's.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 61bfbb9..10be29b 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -108,6 +108,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
 obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
 obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
 obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
-
+obj-$(CONFIG_REGULATOR_ST_FLASHSS) += st-flashss.o
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/st-flashss.c b/drivers/regulator/st-flashss.c
new file mode 100644
index 0000000..9c0e011
--- /dev/null
+++ b/drivers/regulator/st-flashss.c
@@ -0,0 +1,274 @@
+/*
+ * ST regulator driver for flashSS vsense
+ *
+ * This is a small driver to manage the voltage regulator inside the ST flash
+ * sub-system that is used for configuring MMC, NAND, SPI voltages.
+ *
+ * Copyright(C) 2015 STMicroelectronics Ltd
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under  the terms of the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+
+/* FlashSS voltage VSENSE TOP CONFIG register defines */
+#define TOP_VSENSE_CONFIG_REG_PSW_EMMC		BIT(0)
+#define TOP_VSENSE_CONFIG_ENB_REG_PSW_EMMC	BIT(1)
+#define TOP_VSENSE_CONFIG_REG_PSW_NAND		BIT(8)
+#define TOP_VSENSE_CONFIG_ENB_REG_PSW_NAND	BIT(9)
+#define TOP_VSENSE_CONFIG_REG_PSW_SPI		BIT(16)
+#define TOP_VSENSE_CONFIG_ENB_REG_PSW_SPI	BIT(17)
+#define TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC	BIT(24)
+#define TOP_VSENSE_CONFIG_LATCHED_PSW_NAND	BIT(25)
+#define TOP_VSENSE_CONFIG_LATCHED_PSW_SPI	BIT(26)
+
+struct st_vsense {
+	char *name;
+	struct device *dev;
+	u8 n_voltages;			/* number of supported voltages */
+	void __iomem *ioaddr;		/* TOP config base address */
+	unsigned int en_psw_mask;	/* Mask/enable vdd for each device */
+	unsigned int psw_mask;		/* Power sel mask for VDD */
+	unsigned int latched_mask;	/* Latched mask for VDD */
+};
+
+static const unsigned int st_type_voltages[] = {
+	1800000,
+	3300000,
+};
+
+const struct st_vsense st_vsense_data[] = {
+	{
+		.en_psw_mask = TOP_VSENSE_CONFIG_ENB_REG_PSW_EMMC,
+		.psw_mask = TOP_VSENSE_CONFIG_REG_PSW_EMMC,
+		.latched_mask = TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC,
+	}, {
+		.en_psw_mask = TOP_VSENSE_CONFIG_ENB_REG_PSW_NAND,
+		.psw_mask = TOP_VSENSE_CONFIG_REG_PSW_NAND,
+		.latched_mask = TOP_VSENSE_CONFIG_LATCHED_PSW_NAND,
+	}, {
+		.en_psw_mask = TOP_VSENSE_CONFIG_ENB_REG_PSW_SPI,
+		.psw_mask = TOP_VSENSE_CONFIG_REG_PSW_SPI,
+		.latched_mask = TOP_VSENSE_CONFIG_LATCHED_PSW_SPI,
+	},
+};
+
+/* Get the value of Sensed-PSW of eMMC/NAND/SPI Pads */
+static int st_get_voltage_sel(struct regulator_dev *dev)
+{
+	struct st_vsense *vsense = rdev_get_drvdata(dev);
+	void __iomem *ioaddr = vsense->ioaddr;
+	int sel = 0;
+	u32 value = readl_relaxed(ioaddr);
+
+	if (value & vsense->latched_mask)
+		sel = 1;
+
+	dev_dbg(vsense->dev, "%s, selection %d (0x%08x)\n", vsense->name, sel,
+		readl_relaxed(ioaddr));
+
+	return sel;
+}
+
+static int st_set_voltage_sel(struct regulator_dev *dev, unsigned int selector)
+{
+	struct st_vsense *vsense = rdev_get_drvdata(dev);
+	void __iomem *ioaddr = vsense->ioaddr;
+	unsigned int value = readl_relaxed(ioaddr);
+	unsigned int voltage;
+
+	voltage = st_type_voltages[selector];
+
+	value |= vsense->en_psw_mask;
+	if (voltage == 3300000)
+		value |= vsense->psw_mask;
+	else
+		value &= ~vsense->psw_mask;
+
+	writel_relaxed(value, ioaddr);
+
+	dev_dbg(vsense->dev, "%s, required voltage %d (vsense_conf 0x%08x)\n",
+		vsense->name, voltage,
+		readl_relaxed(ioaddr));
+
+	return 0;
+}
+
+static struct regulator_ops st_ops = {
+	.get_voltage_sel = st_get_voltage_sel,
+	.set_voltage_sel = st_set_voltage_sel,
+	.list_voltage = regulator_list_voltage_table,
+};
+
+static const struct of_device_id of_st_match_tbl[] = {
+	{.compatible = "st,vqmmc", .data = &st_vsense_data[0]},
+	{.compatible = "st,vnand", .data = &st_vsense_data[1]},
+	{.compatible = "st,vspi", .data = &st_vsense_data[2]},
+	{ /* end */ }
+};
+
+static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
+{
+	void __iomem *ioaddr = vsense->ioaddr;
+	u32 value = readl_relaxed(ioaddr);
+
+	dev_dbg(vsense->dev, "Initial start-up value: (0x%08x)\n", value);
+
+	/* Sanitize voltage values forcing what is provided from start-up */
+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC)
+		value |= TOP_VSENSE_CONFIG_REG_PSW_EMMC;
+	else
+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;
+
+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND)
+		value |= TOP_VSENSE_CONFIG_REG_PSW_NAND;
+	else
+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;
+
+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI)
+		value |= TOP_VSENSE_CONFIG_REG_PSW_SPI;
+	else
+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;
+
+	writel_relaxed(value, ioaddr);
+
+	dev_dbg(vsense->dev, "Sanitized value: (0x%08x)\n", value);
+}
+
+static int st_vsense_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct st_vsense *vsense;
+	const struct of_device_id *device;
+	struct resource *res;
+	struct regulator_desc *rdesc;
+	struct regulator_dev *rdev;
+	struct regulator_config config = { };
+
+	vsense = devm_kzalloc(dev, sizeof(*vsense), GFP_KERNEL);
+	if (!vsense)
+		return -ENOMEM;
+
+	vsense->dev = dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	vsense->ioaddr = devm_ioremap_resource(dev, res);
+	if (IS_ERR(vsense->ioaddr))
+		return PTR_ERR(vsense->ioaddr);
+
+	rdesc = devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL);
+	if (!rdesc)
+		return -ENOMEM;
+
+	device = of_match_device(of_st_match_tbl, &pdev->dev);
+	if (!device)
+		return -ENODEV;
+
+	if (device->data) {
+		const struct st_vsense *data = device->data;
+
+		vsense->en_psw_mask = data->en_psw_mask;
+		vsense->psw_mask = data->psw_mask;
+		vsense->latched_mask = data->latched_mask;
+	} else
+		return -ENODEV;
+
+	if (of_property_read_string(np, "regulator-name",
+				    (const char **)&vsense->name))
+		return -EINVAL;
+
+	memset(rdesc, 0, sizeof(*rdesc));
+	rdesc->name = vsense->name;
+	rdesc->ops = &st_ops;
+	rdesc->type = REGULATOR_VOLTAGE;
+	rdesc->owner = THIS_MODULE;
+	rdesc->n_voltages = ARRAY_SIZE(st_type_voltages);
+	rdesc->volt_table = st_type_voltages;
+	config.dev = &pdev->dev;
+	config.driver_data = vsense;
+	config.of_node = np;
+
+	config.init_data = of_get_regulator_init_data(&pdev->dev, np, rdesc);
+	if (!config.init_data) {
+		dev_err(dev, "Failed to parse regulator init data\n");
+		return -ENOMEM;
+	}
+
+	/* register regulator */
+	rdev = regulator_register(rdesc, &config);
+	if (IS_ERR(rdev)) {
+		dev_err(dev, "failed to register %s\n", rdesc->name);
+		return PTR_ERR(rdev);
+	}
+
+	platform_set_drvdata(pdev, rdev);
+
+	dev_info(dev, "%s  vsense voltage regulator registered\n", rdesc->name);
+	st_get_satinize_powerup_voltage(vsense);
+
+	return 0;
+}
+
+static int st_vsense_remove(struct platform_device *pdev)
+{
+	struct regulator_dev *rdev = platform_get_drvdata(pdev);
+
+	regulator_unregister(rdev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int st_vsense_resume(struct device *dev)
+{
+	struct regulator_dev *rdev = dev_get_drvdata(dev);
+	struct st_vsense *vsense = rdev_get_drvdata(rdev);
+
+	st_get_satinize_powerup_voltage(vsense);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops st_vsense_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(NULL, st_vsense_resume)
+};
+
+static struct platform_driver st_vsense_driver = {
+	.driver = {
+		   .name = "st-vsense",
+		   .of_match_table = of_st_match_tbl,
+		   .pm	= &st_vsense_pm_ops,
+		   },
+	.probe = st_vsense_probe,
+	.remove = st_vsense_remove,
+};
+
+static int __init st_vsense_init(void)
+{
+	return platform_driver_register(&st_vsense_driver);
+}
+
+subsys_initcall(st_vsense_init);
+
+static void __exit st_vsense_cleanup(void)
+{
+	platform_driver_unregister(&st_vsense_driver);
+}
+
+module_exit(st_vsense_cleanup);
+
+MODULE_AUTHOR("Giuseppe Cavallaro <peppe.cavallaro@st.com>");
+MODULE_DESCRIPTION("ST voltage regulator driver for vsense flashSS");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH 3/5] MAINTAINERS: Add st-flashss driver to STi section.

[Peter Griffin]

This patch adds the st-flashss regulator driver to the STi
section of the maintainers file.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 61a323a..93ef247 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1647,6 +1647,7 @@ F:	drivers/phy/phy-miphy365x.c
 F:	drivers/phy/phy-stih407-usb.c
 F:	drivers/phy/phy-stih41x-usb.c
 F:	drivers/pinctrl/pinctrl-st.c
+F:	drivers/regulator/st-flashss.c
 F:	drivers/reset/sti/
 F:	drivers/rtc/rtc-st-lpc.c
 F:	drivers/tty/serial/st-asc.c
-- 
1.9.1

[PATCH 3/5] MAINTAINERS: Add st-flashss driver to STi section.

[Peter Griffin]

This patch adds the st-flashss regulator driver to the STi
section of the maintainers file.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 61a323a..93ef247 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1647,6 +1647,7 @@ F:	drivers/phy/phy-miphy365x.c
 F:	drivers/phy/phy-stih407-usb.c
 F:	drivers/phy/phy-stih41x-usb.c
 F:	drivers/pinctrl/pinctrl-st.c
+F:	drivers/regulator/st-flashss.c
 F:	drivers/reset/sti/
 F:	drivers/rtc/rtc-st-lpc.c
 F:	drivers/tty/serial/st-asc.c
-- 
1.9.1

[PATCH 4/5] ARM: multi_v7_defconfig: Enable flashss regulator driver.

[Peter Griffin]

Enable the flashss regulator driver found in the flash sub system
on stih407 based silicon.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 2823490..088422b 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -503,6 +503,7 @@ CONFIG_REGULATOR_TPS6586X=y
 CONFIG_REGULATOR_TPS65910=y
 CONFIG_REGULATOR_TWL4030=y
 CONFIG_REGULATOR_VEXPRESS=y
+CONFIG_REGULATOR_ST_FLASHSS=y
 CONFIG_MEDIA_SUPPORT=m
 CONFIG_MEDIA_CAMERA_SUPPORT=y
 CONFIG_MEDIA_CONTROLLER=y
-- 
1.9.1

[PATCH 4/5] ARM: multi_v7_defconfig: Enable flashss regulator driver.

[Peter Griffin]

Enable the flashss regulator driver found in the flash sub system
on stih407 based silicon.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 2823490..088422b 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -503,6 +503,7 @@ CONFIG_REGULATOR_TPS6586X=y
 CONFIG_REGULATOR_TPS65910=y
 CONFIG_REGULATOR_TWL4030=y
 CONFIG_REGULATOR_VEXPRESS=y
+CONFIG_REGULATOR_ST_FLASHSS=y
 CONFIG_MEDIA_SUPPORT=m
 CONFIG_MEDIA_CAMERA_SUPPORT=y
 CONFIG_MEDIA_CONTROLLER=y
-- 
1.9.1

[PATCH 5/5] ARM: STi: DT: STiH407: Add the flashss voltage regulator DT node.

[Peter Griffin]

This patch adds the DT node for the flashss mmc voltage regulator.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/boot/dts/stih407-family.dtsi | 12 ++++++++++++
 arch/arm/boot/dts/stihxxx-b2120.dtsi  |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 81f8121..2d62036 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -488,6 +488,17 @@
 			status = "disabled";
 		};
 
+		vqmmc_reg: voltage-regulator {
+			compatible = "st,vqmmc";
+			regulator-name = "vqmmc0";
+			/* Control register for the VSENSE Module */
+			reg = <0x9061004 0x4>;
+			status = "disabled";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+
 		mmc0: sdhci@09060000 {
 			compatible = "st,sdhci-stih407", "st,sdhci";
 			status = "disabled";
@@ -501,6 +512,7 @@
 			clocks = <&clk_s_c0_flexgen CLK_MMC_0>;
 			bus-width = <8>;
 			non-removable;
+			vqmmc-supply = <&vqmmc_reg>;
 		};
 
 		mmc1: sdhci@09080000 {
diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
index 133375b..f326d57 100644
--- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
+++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
@@ -72,6 +72,10 @@
 			status = "okay";
 		};
 
+		vqmmc_reg: voltage-regulator {
+			status = "okay";
+		};
+
 		/* SSC11 to HDMI */
 		hdmiddc: i2c@9541000 {
 			status = "okay";
-- 
1.9.1

[PATCH 5/5] ARM: STi: DT: STiH407: Add the flashss voltage regulator DT node.

[Peter Griffin]

This patch adds the DT node for the flashss mmc voltage regulator.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/boot/dts/stih407-family.dtsi | 12 ++++++++++++
 arch/arm/boot/dts/stihxxx-b2120.dtsi  |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 81f8121..2d62036 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -488,6 +488,17 @@
 			status = "disabled";
 		};
 
+		vqmmc_reg: voltage-regulator {
+			compatible = "st,vqmmc";
+			regulator-name = "vqmmc0";
+			/* Control register for the VSENSE Module */
+			reg = <0x9061004 0x4>;
+			status = "disabled";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+
 		mmc0: sdhci at 09060000 {
 			compatible = "st,sdhci-stih407", "st,sdhci";
 			status = "disabled";
@@ -501,6 +512,7 @@
 			clocks = <&clk_s_c0_flexgen CLK_MMC_0>;
 			bus-width = <8>;
 			non-removable;
+			vqmmc-supply = <&vqmmc_reg>;
 		};
 
 		mmc1: sdhci at 09080000 {
diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
index 133375b..f326d57 100644
--- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
+++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
@@ -72,6 +72,10 @@
 			status = "okay";
 		};
 
+		vqmmc_reg: voltage-regulator {
+			status = "okay";
+		};
+
 		/* SSC11 to HDMI */
 		hdmiddc: i2c at 9541000 {
 			status = "okay";
-- 
1.9.1

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Mark Brown]

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

On Tue, Apr 12, 2016 at 04:16:41PM +0100, Peter Griffin wrote:

> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 61bfbb9..10be29b 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -108,6 +108,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
>  obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
>  obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>  obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
> -
> +obj-$(CONFIG_REGULATOR_ST_FLASHSS) += st-flashss.o

Please keep Kconfig and Makefile sorted.  :/

> +static int st_set_voltage_sel(struct regulator_dev *dev, unsigned int selector)

This and the get_voltage_sel() look like you can just use a MMIO regmap.

> +	voltage = st_type_voltages[selector];

No bounds checking on the array, though since you're working with
selectors it doesn't seem like there's any need to do this.

> +	value |= vsense->en_psw_mask;

Enable should be a separate operation.

> +static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
> +{
> +	void __iomem *ioaddr = vsense->ioaddr;
> +	u32 value = readl_relaxed(ioaddr);
> +
> +	dev_dbg(vsense->dev, "Initial start-up value: (0x%08x)\n", value);
> +
> +	/* Sanitize voltage values forcing what is provided from start-up */
> +	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC)
> +		value |= TOP_VSENSE_CONFIG_REG_PSW_EMMC;
> +	else
> +		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;
> +
> +	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND)
> +		value |= TOP_VSENSE_CONFIG_REG_PSW_NAND;
> +	else
> +		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;
> +
> +	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI)
> +		value |= TOP_VSENSE_CONFIG_REG_PSW_SPI;
> +	else
> +		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;

This looks like a very complicated way of writing

	value &= TOP_VSENSE_CONFIG_LATCHED_PSW_SPI | 
		TOP_VSENSE_CONFIG_LATCHED_PSW_NAND |
		TOP_VSENSE_CONFIG_REG_PSW_EMMC

or am I missing something?  Why do we need to do this anyway, it's very
surprsing?

> +	if (device->data) {
> +		const struct st_vsense *data = device->data;
> +
> +		vsense->en_psw_mask = data->en_psw_mask;
> +		vsense->psw_mask = data->psw_mask;
> +		vsense->latched_mask = data->latched_mask;
> +	} else
> +		return -ENODEV;

Coding style, { } on both sides.

> +
> +	if (of_property_read_string(np, "regulator-name",
> +				    (const char **)&vsense->name))
> +		return -EINVAL;

Don't make this mandatory, that's not what it's for.  Use the compatible
if you really need something, or just make the framework cope with a
missing name (eg, by using dev_name() or something).

> +	config.init_data = of_get_regulator_init_data(&pdev->dev, np, rdesc);
> +	if (!config.init_data) {
> +		dev_err(dev, "Failed to parse regulator init data\n");
> +		return -ENOMEM;
> +	}

Don't error out, carry on and let the driver work in read only mode for
diagnostics.

> +	/* register regulator */
> +	rdev = regulator_register(rdesc, &config);

devm_regulator_register().

> +	dev_info(dev, "%s  vsense voltage regulator registered\n", rdesc->name);

Remove this, it's just making the boot noisy.

> +static int __init st_vsense_init(void)
> +{
> +	return platform_driver_register(&st_vsense_driver);
> +}
> +
> +subsys_initcall(st_vsense_init);

module_platform_driver().

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

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Mark Brown]

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

On Tue, Apr 12, 2016 at 04:16:41PM +0100, Peter Griffin wrote:

> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 61bfbb9..10be29b 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -108,6 +108,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
>  obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
>  obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>  obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
> -
> +obj-$(CONFIG_REGULATOR_ST_FLASHSS) += st-flashss.o

Please keep Kconfig and Makefile sorted.  :/

> +static int st_set_voltage_sel(struct regulator_dev *dev, unsigned int selector)

This and the get_voltage_sel() look like you can just use a MMIO regmap.

> +	voltage = st_type_voltages[selector];

No bounds checking on the array, though since you're working with
selectors it doesn't seem like there's any need to do this.

> +	value |= vsense->en_psw_mask;

Enable should be a separate operation.

> +static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
> +{
> +	void __iomem *ioaddr = vsense->ioaddr;
> +	u32 value = readl_relaxed(ioaddr);
> +
> +	dev_dbg(vsense->dev, "Initial start-up value: (0x%08x)\n", value);
> +
> +	/* Sanitize voltage values forcing what is provided from start-up */
> +	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC)
> +		value |= TOP_VSENSE_CONFIG_REG_PSW_EMMC;
> +	else
> +		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;
> +
> +	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND)
> +		value |= TOP_VSENSE_CONFIG_REG_PSW_NAND;
> +	else
> +		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;
> +
> +	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI)
> +		value |= TOP_VSENSE_CONFIG_REG_PSW_SPI;
> +	else
> +		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;

This looks like a very complicated way of writing

	value &= TOP_VSENSE_CONFIG_LATCHED_PSW_SPI | 
		TOP_VSENSE_CONFIG_LATCHED_PSW_NAND |
		TOP_VSENSE_CONFIG_REG_PSW_EMMC

or am I missing something?  Why do we need to do this anyway, it's very
surprsing?

> +	if (device->data) {
> +		const struct st_vsense *data = device->data;
> +
> +		vsense->en_psw_mask = data->en_psw_mask;
> +		vsense->psw_mask = data->psw_mask;
> +		vsense->latched_mask = data->latched_mask;
> +	} else
> +		return -ENODEV;

Coding style, { } on both sides.

> +
> +	if (of_property_read_string(np, "regulator-name",
> +				    (const char **)&vsense->name))
> +		return -EINVAL;

Don't make this mandatory, that's not what it's for.  Use the compatible
if you really need something, or just make the framework cope with a
missing name (eg, by using dev_name() or something).

> +	config.init_data = of_get_regulator_init_data(&pdev->dev, np, rdesc);
> +	if (!config.init_data) {
> +		dev_err(dev, "Failed to parse regulator init data\n");
> +		return -ENOMEM;
> +	}

Don't error out, carry on and let the driver work in read only mode for
diagnostics.

> +	/* register regulator */
> +	rdev = regulator_register(rdesc, &config);

devm_regulator_register().

> +	dev_info(dev, "%s  vsense voltage regulator registered\n", rdesc->name);

Remove this, it's just making the boot noisy.

> +static int __init st_vsense_init(void)
> +{
> +	return platform_driver_register(&st_vsense_driver);
> +}
> +
> +subsys_initcall(st_vsense_init);

module_platform_driver().

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

[PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Mark Brown]

On Tue, Apr 12, 2016 at 04:16:41PM +0100, Peter Griffin wrote:

> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 61bfbb9..10be29b 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -108,6 +108,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
>  obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
>  obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>  obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
> -
> +obj-$(CONFIG_REGULATOR_ST_FLASHSS) += st-flashss.o

Please keep Kconfig and Makefile sorted.  :/

> +static int st_set_voltage_sel(struct regulator_dev *dev, unsigned int selector)

This and the get_voltage_sel() look like you can just use a MMIO regmap.

> +	voltage = st_type_voltages[selector];

No bounds checking on the array, though since you're working with
selectors it doesn't seem like there's any need to do this.

> +	value |= vsense->en_psw_mask;

Enable should be a separate operation.

> +static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
> +{
> +	void __iomem *ioaddr = vsense->ioaddr;
> +	u32 value = readl_relaxed(ioaddr);
> +
> +	dev_dbg(vsense->dev, "Initial start-up value: (0x%08x)\n", value);
> +
> +	/* Sanitize voltage values forcing what is provided from start-up */
> +	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC)
> +		value |= TOP_VSENSE_CONFIG_REG_PSW_EMMC;
> +	else
> +		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;
> +
> +	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND)
> +		value |= TOP_VSENSE_CONFIG_REG_PSW_NAND;
> +	else
> +		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;
> +
> +	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI)
> +		value |= TOP_VSENSE_CONFIG_REG_PSW_SPI;
> +	else
> +		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;

This looks like a very complicated way of writing

	value &= TOP_VSENSE_CONFIG_LATCHED_PSW_SPI | 
		TOP_VSENSE_CONFIG_LATCHED_PSW_NAND |
		TOP_VSENSE_CONFIG_REG_PSW_EMMC

or am I missing something?  Why do we need to do this anyway, it's very
surprsing?

> +	if (device->data) {
> +		const struct st_vsense *data = device->data;
> +
> +		vsense->en_psw_mask = data->en_psw_mask;
> +		vsense->psw_mask = data->psw_mask;
> +		vsense->latched_mask = data->latched_mask;
> +	} else
> +		return -ENODEV;

Coding style, { } on both sides.

> +
> +	if (of_property_read_string(np, "regulator-name",
> +				    (const char **)&vsense->name))
> +		return -EINVAL;

Don't make this mandatory, that's not what it's for.  Use the compatible
if you really need something, or just make the framework cope with a
missing name (eg, by using dev_name() or something).

> +	config.init_data = of_get_regulator_init_data(&pdev->dev, np, rdesc);
> +	if (!config.init_data) {
> +		dev_err(dev, "Failed to parse regulator init data\n");
> +		return -ENOMEM;
> +	}

Don't error out, carry on and let the driver work in read only mode for
diagnostics.

> +	/* register regulator */
> +	rdev = regulator_register(rdesc, &config);

devm_regulator_register().

> +	dev_info(dev, "%s  vsense voltage regulator registered\n", rdesc->name);

Remove this, it's just making the boot noisy.

> +static int __init st_vsense_init(void)
> +{
> +	return platform_driver_register(&st_vsense_driver);
> +}
> +
> +subsys_initcall(st_vsense_init);

module_platform_driver().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160413/e7c0ce72/attachment.sig>

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Giuseppe CAVALLARO]

On 4/13/2016 8:15 AM, Mark Brown wrote:
>> >+static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
>> >+{
>> >+	void __iomem *ioaddr = vsense->ioaddr;
>> >+	u32 value = readl_relaxed(ioaddr);
>> >+
>> >+	dev_dbg(vsense->dev, "Initial start-up value: (0x%08x)\n", value);
>> >+
>> >+	/* Sanitize voltage values forcing what is provided from start-up */
>> >+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC)
>> >+		value |= TOP_VSENSE_CONFIG_REG_PSW_EMMC;
>> >+	else
>> >+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;
>> >+
>> >+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND)
>> >+		value |= TOP_VSENSE_CONFIG_REG_PSW_NAND;
>> >+	else
>> >+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;
>> >+
>> >+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI)
>> >+		value |= TOP_VSENSE_CONFIG_REG_PSW_SPI;
>> >+	else
>> >+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;
> This looks like a very complicated way of writing
>
> 	value &= TOP_VSENSE_CONFIG_LATCHED_PSW_SPI |
> 		TOP_VSENSE_CONFIG_LATCHED_PSW_NAND |
> 		TOP_VSENSE_CONFIG_REG_PSW_EMMC
>
> or am I missing something?  Why do we need to do this anyway, it's very
> surprsing?
>

This functions is to sanitize the vsense voltages when the regulator
is probed and in some circumstances the reset value of this register
does not reflect the hw status/config. For example, by default, after
the reset, the bit 0 is set so the EMMC, inside the flash subsystem,
is supposed to operate at 3v3. But the latched bit 24 can be 0 on
a platform where it is actually set at 1v8.
So the bit 0 must be reset to keep this coherent and to allow MMC
framework to properly setup the Vdd when the framework starts.

Hoping this can help.

Regards
peppe

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Giuseppe CAVALLARO]

On 4/13/2016 8:15 AM, Mark Brown wrote:
>> >+static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
>> >+{
>> >+	void __iomem *ioaddr = vsense->ioaddr;
>> >+	u32 value = readl_relaxed(ioaddr);
>> >+
>> >+	dev_dbg(vsense->dev, "Initial start-up value: (0x%08x)\n", value);
>> >+
>> >+	/* Sanitize voltage values forcing what is provided from start-up */
>> >+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC)
>> >+		value |= TOP_VSENSE_CONFIG_REG_PSW_EMMC;
>> >+	else
>> >+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;
>> >+
>> >+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND)
>> >+		value |= TOP_VSENSE_CONFIG_REG_PSW_NAND;
>> >+	else
>> >+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;
>> >+
>> >+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI)
>> >+		value |= TOP_VSENSE_CONFIG_REG_PSW_SPI;
>> >+	else
>> >+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;
> This looks like a very complicated way of writing
>
> 	value &= TOP_VSENSE_CONFIG_LATCHED_PSW_SPI |
> 		TOP_VSENSE_CONFIG_LATCHED_PSW_NAND |
> 		TOP_VSENSE_CONFIG_REG_PSW_EMMC
>
> or am I missing something?  Why do we need to do this anyway, it's very
> surprsing?
>

This functions is to sanitize the vsense voltages when the regulator
is probed and in some circumstances the reset value of this register
does not reflect the hw status/config. For example, by default, after
the reset, the bit 0 is set so the EMMC, inside the flash subsystem,
is supposed to operate at 3v3. But the latched bit 24 can be 0 on
a platform where it is actually set at 1v8.
So the bit 0 must be reset to keep this coherent and to allow MMC
framework to properly setup the Vdd when the framework starts.

Hoping this can help.

Regards
peppe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Giuseppe CAVALLARO]

On 4/13/2016 8:15 AM, Mark Brown wrote:
>> >+static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
>> >+{
>> >+	void __iomem *ioaddr = vsense->ioaddr;
>> >+	u32 value = readl_relaxed(ioaddr);
>> >+
>> >+	dev_dbg(vsense->dev, "Initial start-up value: (0x%08x)\n", value);
>> >+
>> >+	/* Sanitize voltage values forcing what is provided from start-up */
>> >+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC)
>> >+		value |= TOP_VSENSE_CONFIG_REG_PSW_EMMC;
>> >+	else
>> >+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;
>> >+
>> >+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND)
>> >+		value |= TOP_VSENSE_CONFIG_REG_PSW_NAND;
>> >+	else
>> >+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;
>> >+
>> >+	if (value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI)
>> >+		value |= TOP_VSENSE_CONFIG_REG_PSW_SPI;
>> >+	else
>> >+		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;
> This looks like a very complicated way of writing
>
> 	value &= TOP_VSENSE_CONFIG_LATCHED_PSW_SPI |
> 		TOP_VSENSE_CONFIG_LATCHED_PSW_NAND |
> 		TOP_VSENSE_CONFIG_REG_PSW_EMMC
>
> or am I missing something?  Why do we need to do this anyway, it's very
> surprsing?
>

This functions is to sanitize the vsense voltages when the regulator
is probed and in some circumstances the reset value of this register
does not reflect the hw status/config. For example, by default, after
the reset, the bit 0 is set so the EMMC, inside the flash subsystem,
is supposed to operate at 3v3. But the latched bit 24 can be 0 on
a platform where it is actually set at 1v8.
So the bit 0 must be reset to keep this coherent and to allow MMC
framework to properly setup the Vdd when the framework starts.

Hoping this can help.

Regards
peppe

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Mark Brown]

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

On Wed, Apr 13, 2016 at 09:59:00AM +0200, Giuseppe CAVALLARO wrote:
> On 4/13/2016 8:15 AM, Mark Brown wrote:
> >>>+static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
> >>>+{

> >or am I missing something?  Why do we need to do this anyway, it's very
> >surprsing?

> This functions is to sanitize the vsense voltages when the regulator
> is probed and in some circumstances the reset value of this register
> does not reflect the hw status/config. For example, by default, after
> the reset, the bit 0 is set so the EMMC, inside the flash subsystem,
> is supposed to operate at 3v3. But the latched bit 24 can be 0 on
> a platform where it is actually set at 1v8.
> So the bit 0 must be reset to keep this coherent and to allow MMC
> framework to properly setup the Vdd when the framework starts.

I'm afraid I can't follow that explanation, perhaps because I don't know
anything about the content of this register except for these three bits.
I think we do need a comment in the driver explaining what's going on,
and probably a simplification of the code too if my understanding of the
effect of all those operations is correct.

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

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Mark Brown]

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

On Wed, Apr 13, 2016 at 09:59:00AM +0200, Giuseppe CAVALLARO wrote:
> On 4/13/2016 8:15 AM, Mark Brown wrote:
> >>>+static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
> >>>+{

> >or am I missing something?  Why do we need to do this anyway, it's very
> >surprsing?

> This functions is to sanitize the vsense voltages when the regulator
> is probed and in some circumstances the reset value of this register
> does not reflect the hw status/config. For example, by default, after
> the reset, the bit 0 is set so the EMMC, inside the flash subsystem,
> is supposed to operate at 3v3. But the latched bit 24 can be 0 on
> a platform where it is actually set at 1v8.
> So the bit 0 must be reset to keep this coherent and to allow MMC
> framework to properly setup the Vdd when the framework starts.

I'm afraid I can't follow that explanation, perhaps because I don't know
anything about the content of this register except for these three bits.
I think we do need a comment in the driver explaining what's going on,
and probably a simplification of the code too if my understanding of the
effect of all those operations is correct.

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

[PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Mark Brown]

On Wed, Apr 13, 2016 at 09:59:00AM +0200, Giuseppe CAVALLARO wrote:
> On 4/13/2016 8:15 AM, Mark Brown wrote:
> >>>+static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
> >>>+{

> >or am I missing something?  Why do we need to do this anyway, it's very
> >surprsing?

> This functions is to sanitize the vsense voltages when the regulator
> is probed and in some circumstances the reset value of this register
> does not reflect the hw status/config. For example, by default, after
> the reset, the bit 0 is set so the EMMC, inside the flash subsystem,
> is supposed to operate at 3v3. But the latched bit 24 can be 0 on
> a platform where it is actually set at 1v8.
> So the bit 0 must be reset to keep this coherent and to allow MMC
> framework to properly setup the Vdd when the framework starts.

I'm afraid I can't follow that explanation, perhaps because I don't know
anything about the content of this register except for these three bits.
I think we do need a comment in the driver explaining what's going on,
and probably a simplification of the code too if my understanding of the
effect of all those operations is correct.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160413/af3aaa8a/attachment.sig>

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Giuseppe CAVALLARO]

Hello Mark

On 4/13/2016 7:23 PM, Mark Brown wrote:
> On Wed, Apr 13, 2016 at 09:59:00AM +0200, Giuseppe CAVALLARO wrote:
>> On 4/13/2016 8:15 AM, Mark Brown wrote:
>>>>> +static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
>>>>> +{
>
>>> or am I missing something?  Why do we need to do this anyway, it's very
>>> surprsing?
>
>> This functions is to sanitize the vsense voltages when the regulator
>> is probed and in some circumstances the reset value of this register
>> does not reflect the hw status/config. For example, by default, after
>> the reset, the bit 0 is set so the EMMC, inside the flash subsystem,
>> is supposed to operate at 3v3. But the latched bit 24 can be 0 on
>> a platform where it is actually set at 1v8.
>> So the bit 0 must be reset to keep this coherent and to allow MMC
>> framework to properly setup the Vdd when the framework starts.
>
> I'm afraid I can't follow that explanation, perhaps because I don't know
> anything about the content of this register except for these three bits.
> I think we do need a comment in the driver explaining what's going on,

yes you are right

> and probably a simplification of the code too if my understanding of the
> effect of all those operations is correct.

Maybe we can simplify the function as:

static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
{
	void __iomem *ioaddr = vsense->ioaddr;
	u32 value = readl_relaxed(ioaddr);

	/*
	 * After resetting, the CONFIG_REG_PSW_<xxx> are set, this
	 * means 3v3 operating voltage.
	 * The CONFIG_LATCHED_PSW_<xxx> must be used to fix the previous
	 * bits so operating at 1v8 if this is the real HW configuration
	 * at boot time.
	 */
	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC))
		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;

	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND))
		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;

	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI))
		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;

	writel_relaxed(value, ioaddr);
}

Le me know if this looks a bit more clear.

Peppe

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Giuseppe CAVALLARO]

Hello Mark

On 4/13/2016 7:23 PM, Mark Brown wrote:
> On Wed, Apr 13, 2016 at 09:59:00AM +0200, Giuseppe CAVALLARO wrote:
>> On 4/13/2016 8:15 AM, Mark Brown wrote:
>>>>> +static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
>>>>> +{
>
>>> or am I missing something?  Why do we need to do this anyway, it's very
>>> surprsing?
>
>> This functions is to sanitize the vsense voltages when the regulator
>> is probed and in some circumstances the reset value of this register
>> does not reflect the hw status/config. For example, by default, after
>> the reset, the bit 0 is set so the EMMC, inside the flash subsystem,
>> is supposed to operate at 3v3. But the latched bit 24 can be 0 on
>> a platform where it is actually set at 1v8.
>> So the bit 0 must be reset to keep this coherent and to allow MMC
>> framework to properly setup the Vdd when the framework starts.
>
> I'm afraid I can't follow that explanation, perhaps because I don't know
> anything about the content of this register except for these three bits.
> I think we do need a comment in the driver explaining what's going on,

yes you are right

> and probably a simplification of the code too if my understanding of the
> effect of all those operations is correct.

Maybe we can simplify the function as:

static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
{
	void __iomem *ioaddr = vsense->ioaddr;
	u32 value = readl_relaxed(ioaddr);

	/*
	 * After resetting, the CONFIG_REG_PSW_<xxx> are set, this
	 * means 3v3 operating voltage.
	 * The CONFIG_LATCHED_PSW_<xxx> must be used to fix the previous
	 * bits so operating at 1v8 if this is the real HW configuration
	 * at boot time.
	 */
	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC))
		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;

	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND))
		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;

	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI))
		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;

	writel_relaxed(value, ioaddr);
}

Le me know if this looks a bit more clear.

Peppe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Giuseppe CAVALLARO]

Hello Mark

On 4/13/2016 7:23 PM, Mark Brown wrote:
> On Wed, Apr 13, 2016 at 09:59:00AM +0200, Giuseppe CAVALLARO wrote:
>> On 4/13/2016 8:15 AM, Mark Brown wrote:
>>>>> +static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
>>>>> +{
>
>>> or am I missing something?  Why do we need to do this anyway, it's very
>>> surprsing?
>
>> This functions is to sanitize the vsense voltages when the regulator
>> is probed and in some circumstances the reset value of this register
>> does not reflect the hw status/config. For example, by default, after
>> the reset, the bit 0 is set so the EMMC, inside the flash subsystem,
>> is supposed to operate at 3v3. But the latched bit 24 can be 0 on
>> a platform where it is actually set at 1v8.
>> So the bit 0 must be reset to keep this coherent and to allow MMC
>> framework to properly setup the Vdd when the framework starts.
>
> I'm afraid I can't follow that explanation, perhaps because I don't know
> anything about the content of this register except for these three bits.
> I think we do need a comment in the driver explaining what's going on,

yes you are right

> and probably a simplification of the code too if my understanding of the
> effect of all those operations is correct.

Maybe we can simplify the function as:

static void st_get_satinize_powerup_voltage(struct st_vsense *vsense)
{
	void __iomem *ioaddr = vsense->ioaddr;
	u32 value = readl_relaxed(ioaddr);

	/*
	 * After resetting, the CONFIG_REG_PSW_<xxx> are set, this
	 * means 3v3 operating voltage.
	 * The CONFIG_LATCHED_PSW_<xxx> must be used to fix the previous
	 * bits so operating at 1v8 if this is the real HW configuration
	 * at boot time.
	 */
	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC))
		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;

	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_NAND))
		value &= ~TOP_VSENSE_CONFIG_REG_PSW_NAND;

	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_SPI))
		value &= ~TOP_VSENSE_CONFIG_REG_PSW_SPI;

	writel_relaxed(value, ioaddr);
}

Le me know if this looks a bit more clear.

Peppe

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Mark Brown]

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

On Thu, Apr 14, 2016 at 04:15:34PM +0200, Giuseppe CAVALLARO wrote:
> On 4/13/2016 7:23 PM, Mark Brown wrote:

> 	/*
> 	 * After resetting, the CONFIG_REG_PSW_<xxx> are set, this
> 	 * means 3v3 operating voltage.
> 	 * The CONFIG_LATCHED_PSW_<xxx> must be used to fix the previous
> 	 * bits so operating at 1v8 if this is the real HW configuration
> 	 * at boot time.
> 	 */
> 	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC))
> 		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;

That's definitely better.

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

Re: [PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Mark Brown]

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

On Thu, Apr 14, 2016 at 04:15:34PM +0200, Giuseppe CAVALLARO wrote:
> On 4/13/2016 7:23 PM, Mark Brown wrote:

> 	/*
> 	 * After resetting, the CONFIG_REG_PSW_<xxx> are set, this
> 	 * means 3v3 operating voltage.
> 	 * The CONFIG_LATCHED_PSW_<xxx> must be used to fix the previous
> 	 * bits so operating at 1v8 if this is the real HW configuration
> 	 * at boot time.
> 	 */
> 	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC))
> 		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;

That's definitely better.

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

[PATCH 2/5] regulator: st-flashss: Add a regulator driver for flashss vsense.

[Mark Brown]

On Thu, Apr 14, 2016 at 04:15:34PM +0200, Giuseppe CAVALLARO wrote:
> On 4/13/2016 7:23 PM, Mark Brown wrote:

> 	/*
> 	 * After resetting, the CONFIG_REG_PSW_<xxx> are set, this
> 	 * means 3v3 operating voltage.
> 	 * The CONFIG_LATCHED_PSW_<xxx> must be used to fix the previous
> 	 * bits so operating at 1v8 if this is the real HW configuration
> 	 * at boot time.
> 	 */
> 	if (!(value & TOP_VSENSE_CONFIG_LATCHED_PSW_EMMC))
> 		value &= ~TOP_VSENSE_CONFIG_REG_PSW_EMMC;

That's definitely better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160414/4ebe1c40/attachment-0001.sig>

Re: [PATCH 1/5] regulator: st-flashss: Add DT binding documentation for flashss regulator.

[Rob Herring]

On Tue, Apr 12, 2016 at 04:16:40PM +0100, Peter Griffin wrote:
> This patch adds the DT doc for the flashss regulator driver which is
> found in stih407 based silicon.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  .../devicetree/bindings/regulator/st-flashss.txt   | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/st-flashss.txt
> 
> diff --git a/Documentation/devicetree/bindings/regulator/st-flashss.txt b/Documentation/devicetree/bindings/regulator/st-flashss.txt
> new file mode 100644
> index 0000000..04324d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/st-flashss.txt
> @@ -0,0 +1,43 @@
> +ST Voltage regulator for FlashSS vsense
> +
> +This documents the voltage regulator driver which manages the vsense inside the ST flash

Bindings document h/w, not drivers.
> +sub-system that is used for configuring MMC, NAND, SPI voltages.
> +
> +Required properties:
> +- compatible:		Can be "st,vqmmc", "st,vnand" or "st,vspi" because it is shared
> +			among these devices inside the ST FlashSS.
> +- regulator-name:	Regulator name.
> +- reg:			The vsense top config base address in the flashSS hardware.
> +
> +Any property defined as part of the core regulator binding, in
> +Documentation/devicetree/bindings/regulator/regulator.txt can also be used.
> +
> +Example:
> +
> +vqmmc_reg: voltage-regulator {
> +	compatible = "st,vqmmc";
> +	regulator-name = "vqmmc0";
> +	/* Control register for the VSENSE Module */

I'm having a hard time understanding the relationship of FlashSS, VSENSE 
and the compatible strings. The binding looks suspect to me though. If 
VSENSE is a module, then I'd expect a compatible string for it.

Rob

[PATCH 1/5] regulator: st-flashss: Add DT binding documentation for flashss regulator.

[Rob Herring]

On Tue, Apr 12, 2016 at 04:16:40PM +0100, Peter Griffin wrote:
> This patch adds the DT doc for the flashss regulator driver which is
> found in stih407 based silicon.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  .../devicetree/bindings/regulator/st-flashss.txt   | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/st-flashss.txt
> 
> diff --git a/Documentation/devicetree/bindings/regulator/st-flashss.txt b/Documentation/devicetree/bindings/regulator/st-flashss.txt
> new file mode 100644
> index 0000000..04324d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/st-flashss.txt
> @@ -0,0 +1,43 @@
> +ST Voltage regulator for FlashSS vsense
> +
> +This documents the voltage regulator driver which manages the vsense inside the ST flash

Bindings document h/w, not drivers.
> +sub-system that is used for configuring MMC, NAND, SPI voltages.
> +
> +Required properties:
> +- compatible:		Can be "st,vqmmc", "st,vnand" or "st,vspi" because it is shared
> +			among these devices inside the ST FlashSS.
> +- regulator-name:	Regulator name.
> +- reg:			The vsense top config base address in the flashSS hardware.
> +
> +Any property defined as part of the core regulator binding, in
> +Documentation/devicetree/bindings/regulator/regulator.txt can also be used.
> +
> +Example:
> +
> +vqmmc_reg: voltage-regulator {
> +	compatible = "st,vqmmc";
> +	regulator-name = "vqmmc0";
> +	/* Control register for the VSENSE Module */

I'm having a hard time understanding the relationship of FlashSS, VSENSE 
and the compatible strings. The binding looks suspect to me though. If 
VSENSE is a module, then I'd expect a compatible string for it.

Rob