[PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator

[PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator

[John Crispin]

Based on the MT6397 binding documentation.

Signed-off-by: John Crispin <blogic@openwrt.org>
Cc: devicetree@vger.kernel.org
---
 .../bindings/regulator/mt6323-regulator.txt        |  241 ++++++++++++++++++++
 1 file changed, 241 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/mt6323-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/mt6323-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
new file mode 100644
index 0000000..4412102
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
@@ -0,0 +1,241 @@
+Mediatek MT6323 Regulator Driver
+
+Required properties:
+- compatible: "mediatek,mt6323-regulator"
+- mt6323regulator: List of regulators provided by this controller. It is named
+  according to its regulator type, buck_<name> and ldo_<name>.
+  The definition for each of these nodes is defined using the standard binding
+  for regulators at Documentation/devicetree/bindings/regulator/regulator.txt.
+
+The valid names for regulators are::
+BUCK:
+  buck_vproc, buck_vsys, buck_vpa
+LDO:
+  ldo_vtcxo, ldo_vcn28, ldo_vcn33_bt, ldo_vcn33_wifi, ldo_va, ldo_vcama,
+  ldo_vio28, ldo_vusb, ldo_vmc, ldo_vmch, ldo_vemc3v3, ldo_vgp1, ldo_vgp2,
+  ldo_vgp3, ldo_vcn18, ldo_vsim1, ldo_vsim2, ldo_vrtc, ldo_vcamaf, ldo_vibr,
+  ldo_vrf18, ldo_vm, ldo_vio18, ldo_vcamd, ldo_vcamio
+
+Example:
+
+	pmic: mt6323 {
+		compatible = "mediatek,mt6323";
+
+		mt6323regulator: mt6323regulator{
+			compatible = "mediatek,mt6323-regulator";
+
+			mt6323_vproc_reg: buck_vproc{
+				regulator-name = "vproc";
+				regulator-min-microvolt = < 700000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <12500>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vsys_reg: buck_vsys{
+				regulator-name = "vsys";
+				regulator-min-microvolt = <1400000>;
+				regulator-max-microvolt = <2987500>;
+				regulator-ramp-delay = <25000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vpa_reg: buck_vpa{
+				regulator-name = "vpa";
+				regulator-min-microvolt = < 500000>;
+				regulator-max-microvolt = <3650000>;
+			};
+
+			mt6323_vtcxo_reg: ldo_vtcxo{
+				regulator-name = "vtcxo";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <90>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vcn28_reg: ldo_vcn28{
+				regulator-name = "vcn28";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <185>;
+			};
+
+			mt6323_vcn33_bt_reg: ldo_vcn33_bt{
+				regulator-name = "vcn33_bt";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3600000>;
+				regulator-enable-ramp-delay = <185>;
+			};
+
+			mt6323_vcn33_wifi_reg: ldo_vcn33_wifi{
+				regulator-name = "vcn33_wifi";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3600000>;
+				regulator-enable-ramp-delay = <185>;
+			};
+
+			mt6323_va_reg: ldo_va{
+				regulator-name = "va";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vcama_reg: ldo_vcama{
+				regulator-name = "vcama";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vio28_reg: ldo_vio28{
+				regulator-name = "vio28";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vusb_reg: ldo_vusb{
+				regulator-name = "vusb";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-boot-on;
+			};
+
+			mt6323_vmc_reg: ldo_vmc{
+				regulator-name = "vmc";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <36>;
+				regulator-boot-on;
+			};
+
+			mt6323_vmch_reg: ldo_vmch{
+				regulator-name = "vmch";
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <36>;
+				regulator-boot-on;
+			};
+
+			mt6323_vemc3v3_reg: ldo_vemc3v3{
+				regulator-name = "vemc3v3";
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <36>;
+				regulator-boot-on;
+			};
+
+			mt6323_vgp1_reg: ldo_vgp1{
+				regulator-name = "vgp1";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vgp2_reg: ldo_vgp2{
+				regulator-name = "vgp2";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vgp3_reg: ldo_vgp3{
+				regulator-name = "vgp3";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vcn18_reg: ldo_vcn18{
+				regulator-name = "vcn18";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vsim1_reg: ldo_vsim1{
+				regulator-name = "vsim1";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vsim2_reg: ldo_vsim2{
+				regulator-name = "vsim2";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vrtc_reg: ldo_vrtc{
+				regulator-name = "vrtc";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vcamaf_reg: ldo_vcamaf{
+				regulator-name = "vcamaf";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vibr_reg: ldo_vibr{
+				regulator-name = "vibr";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <36>;
+			};
+
+			mt6323_vrf18_reg: ldo_vrf18{
+				regulator-name = "vrf18";
+				regulator-min-microvolt = <1825000>;
+				regulator-max-microvolt = <1825000>;
+				regulator-enable-ramp-delay = <187>;
+			};
+
+			mt6323_vm_reg: ldo_vm{
+				regulator-name = "vm";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vio18_reg: ldo_vio18{
+				regulator-name = "vio18";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vcamd_reg: ldo_vcamd{
+				regulator-name = "vcamd";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vcamio_reg: ldo_vcamio{
+				regulator-name = "vcamio";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+		};
+	};
-- 
1.7.10.4

[PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator

[John Crispin]

Based on the MT6397 binding documentation.

Signed-off-by: John Crispin <blogic@openwrt.org>
Cc: devicetree at vger.kernel.org
---
 .../bindings/regulator/mt6323-regulator.txt        |  241 ++++++++++++++++++++
 1 file changed, 241 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/mt6323-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/mt6323-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
new file mode 100644
index 0000000..4412102
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
@@ -0,0 +1,241 @@
+Mediatek MT6323 Regulator Driver
+
+Required properties:
+- compatible: "mediatek,mt6323-regulator"
+- mt6323regulator: List of regulators provided by this controller. It is named
+  according to its regulator type, buck_<name> and ldo_<name>.
+  The definition for each of these nodes is defined using the standard binding
+  for regulators at Documentation/devicetree/bindings/regulator/regulator.txt.
+
+The valid names for regulators are::
+BUCK:
+  buck_vproc, buck_vsys, buck_vpa
+LDO:
+  ldo_vtcxo, ldo_vcn28, ldo_vcn33_bt, ldo_vcn33_wifi, ldo_va, ldo_vcama,
+  ldo_vio28, ldo_vusb, ldo_vmc, ldo_vmch, ldo_vemc3v3, ldo_vgp1, ldo_vgp2,
+  ldo_vgp3, ldo_vcn18, ldo_vsim1, ldo_vsim2, ldo_vrtc, ldo_vcamaf, ldo_vibr,
+  ldo_vrf18, ldo_vm, ldo_vio18, ldo_vcamd, ldo_vcamio
+
+Example:
+
+	pmic: mt6323 {
+		compatible = "mediatek,mt6323";
+
+		mt6323regulator: mt6323regulator{
+			compatible = "mediatek,mt6323-regulator";
+
+			mt6323_vproc_reg: buck_vproc{
+				regulator-name = "vproc";
+				regulator-min-microvolt = < 700000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <12500>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vsys_reg: buck_vsys{
+				regulator-name = "vsys";
+				regulator-min-microvolt = <1400000>;
+				regulator-max-microvolt = <2987500>;
+				regulator-ramp-delay = <25000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vpa_reg: buck_vpa{
+				regulator-name = "vpa";
+				regulator-min-microvolt = < 500000>;
+				regulator-max-microvolt = <3650000>;
+			};
+
+			mt6323_vtcxo_reg: ldo_vtcxo{
+				regulator-name = "vtcxo";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <90>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vcn28_reg: ldo_vcn28{
+				regulator-name = "vcn28";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <185>;
+			};
+
+			mt6323_vcn33_bt_reg: ldo_vcn33_bt{
+				regulator-name = "vcn33_bt";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3600000>;
+				regulator-enable-ramp-delay = <185>;
+			};
+
+			mt6323_vcn33_wifi_reg: ldo_vcn33_wifi{
+				regulator-name = "vcn33_wifi";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3600000>;
+				regulator-enable-ramp-delay = <185>;
+			};
+
+			mt6323_va_reg: ldo_va{
+				regulator-name = "va";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vcama_reg: ldo_vcama{
+				regulator-name = "vcama";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vio28_reg: ldo_vio28{
+				regulator-name = "vio28";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vusb_reg: ldo_vusb{
+				regulator-name = "vusb";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-boot-on;
+			};
+
+			mt6323_vmc_reg: ldo_vmc{
+				regulator-name = "vmc";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <36>;
+				regulator-boot-on;
+			};
+
+			mt6323_vmch_reg: ldo_vmch{
+				regulator-name = "vmch";
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <36>;
+				regulator-boot-on;
+			};
+
+			mt6323_vemc3v3_reg: ldo_vemc3v3{
+				regulator-name = "vemc3v3";
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <36>;
+				regulator-boot-on;
+			};
+
+			mt6323_vgp1_reg: ldo_vgp1{
+				regulator-name = "vgp1";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vgp2_reg: ldo_vgp2{
+				regulator-name = "vgp2";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vgp3_reg: ldo_vgp3{
+				regulator-name = "vgp3";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vcn18_reg: ldo_vcn18{
+				regulator-name = "vcn18";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vsim1_reg: ldo_vsim1{
+				regulator-name = "vsim1";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vsim2_reg: ldo_vsim2{
+				regulator-name = "vsim2";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vrtc_reg: ldo_vrtc{
+				regulator-name = "vrtc";
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vcamaf_reg: ldo_vcamaf{
+				regulator-name = "vcamaf";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vibr_reg: ldo_vibr{
+				regulator-name = "vibr";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <36>;
+			};
+
+			mt6323_vrf18_reg: ldo_vrf18{
+				regulator-name = "vrf18";
+				regulator-min-microvolt = <1825000>;
+				regulator-max-microvolt = <1825000>;
+				regulator-enable-ramp-delay = <187>;
+			};
+
+			mt6323_vm_reg: ldo_vm{
+				regulator-name = "vm";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vio18_reg: ldo_vio18{
+				regulator-name = "vio18";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			mt6323_vcamd_reg: ldo_vcamd{
+				regulator-name = "vcamd";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+
+			mt6323_vcamio_reg: ldo_vcamio{
+				regulator-name = "vcamio";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-enable-ramp-delay = <216>;
+			};
+		};
+	};
-- 
1.7.10.4

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

From: Chen Zhong <chen.zhong@mediatek.com>

The MT6323 is a regulator found on boards based on MediaTek MT7623 and
probably other SoCs. It is a so called pmic and connects as a slave to
SoC using SPI, wrapped inside the pmic-wrapper.

Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
Signed-off-by: John Crispin <blogic@openwrt.org>
---
 drivers/regulator/Kconfig                  |    9 +
 drivers/regulator/Makefile                 |    1 +
 drivers/regulator/mt6323-regulator.c       |  433 ++++++++++++++++++++++++++++
 include/linux/regulator/mt6323-regulator.h |   52 ++++
 4 files changed, 495 insertions(+)
 create mode 100644 drivers/regulator/mt6323-regulator.c
 create mode 100644 include/linux/regulator/mt6323-regulator.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 8155e80..5e49568 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -462,6 +462,15 @@ config REGULATOR_MT6311
 	  This driver supports the control of different power rails of device
 	  through regulator interface.
 
+config REGULATOR_MT6323
+	tristate "MediaTek MT6323 PMIC"
+	depends on MFD_MT6397
+	help
+	  Say y here to select this option to enable the power regulator of
+	  MediaTek MT6323 PMIC.
+	  This driver supports the control of different power rails of device
+	  through regulator interface.
+
 config REGULATOR_MT6397
 	tristate "MediaTek MT6397 PMIC"
 	depends on MFD_MT6397
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 980b194..8fb55640 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
 obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
 obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
 obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o
+obj-$(CONFIG_REGULATOR_MT6323)	+= mt6323-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
diff --git a/drivers/regulator/mt6323-regulator.c b/drivers/regulator/mt6323-regulator.c
new file mode 100644
index 0000000..1d2621e
--- /dev/null
+++ b/drivers/regulator/mt6323-regulator.c
@@ -0,0 +1,433 @@
+/*
+ * Copyright (c) 2016 MediaTek Inc.
+ * Author: Chen Zhong <chen.zhong@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/mt6397/core.h>
+#include <linux/mfd/mt6323/registers.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/mt6323-regulator.h>
+#include <linux/regulator/of_regulator.h>
+
+#define MT6323_LDO_MODE_NORMAL	0
+#define MT6323_LDO_MODE_LP	1
+
+/*
+ * MT6323 regulators' information
+ *
+ * @desc: standard fields of regulator description.
+ * @qi: Mask for query enable signal status of regulators
+ * @vselon_reg: Register sections for hardware control mode of bucks
+ * @vselctrl_reg: Register for controlling the buck control mode.
+ * @vselctrl_mask: Mask for query buck's voltage control mode.
+ */
+struct mt6323_regulator_info {
+	struct regulator_desc desc;
+	u32 qi;
+	u32 vselon_reg;
+	u32 vselctrl_reg;
+	u32 vselctrl_mask;
+	u32 modeset_reg;
+	u32 modeset_mask;
+};
+
+#define MT6323_BUCK(match, vreg, min, max, step, volt_ranges, enreg,	\
+		vosel, vosel_mask, voselon, vosel_ctrl)			\
+[MT6323_ID_##vreg] = {							\
+	.desc = {							\
+		.name = #vreg,						\
+		.of_match = of_match_ptr(match),			\
+		.ops = &mt6323_volt_range_ops,				\
+		.type = REGULATOR_VOLTAGE,				\
+		.id = MT6323_ID_##vreg,					\
+		.owner = THIS_MODULE,					\
+		.n_voltages = (max - min)/step + 1,			\
+		.linear_ranges = volt_ranges,				\
+		.n_linear_ranges = ARRAY_SIZE(volt_ranges),		\
+		.vsel_reg = vosel,					\
+		.vsel_mask = vosel_mask,				\
+		.enable_reg = enreg,					\
+		.enable_mask = BIT(0),					\
+	},								\
+	.qi = BIT(13),							\
+	.vselon_reg = voselon,						\
+	.vselctrl_reg = vosel_ctrl,					\
+	.vselctrl_mask = BIT(1),					\
+}
+
+#define MT6323_LDO(match, vreg, ldo_volt_table, enreg, enbit, vosel,	\
+		vosel_mask, _modeset_reg, _modeset_mask)		\
+[MT6323_ID_##vreg] = {							\
+	.desc = {							\
+		.name = #vreg,						\
+		.of_match = of_match_ptr(match),			\
+		.ops = &mt6323_volt_table_ops,				\
+		.type = REGULATOR_VOLTAGE,				\
+		.id = MT6323_ID_##vreg,					\
+		.owner = THIS_MODULE,					\
+		.n_voltages = ARRAY_SIZE(ldo_volt_table),		\
+		.volt_table = ldo_volt_table,				\
+		.vsel_reg = vosel,					\
+		.vsel_mask = vosel_mask,				\
+		.enable_reg = enreg,					\
+		.enable_mask = BIT(enbit),				\
+	},								\
+	.qi = BIT(15),							\
+	.modeset_reg = _modeset_reg,					\
+	.modeset_mask = _modeset_mask,					\
+}
+
+#define MT6323_REG_FIXED(match, vreg, enreg, enbit, volt,		\
+		_modeset_reg, _modeset_mask)				\
+[MT6323_ID_##vreg] = {							\
+	.desc = {							\
+		.name = #vreg,						\
+		.of_match = of_match_ptr(match),			\
+		.ops = &mt6323_volt_fixed_ops,				\
+		.type = REGULATOR_VOLTAGE,				\
+		.id = MT6323_ID_##vreg,					\
+		.owner = THIS_MODULE,					\
+		.n_voltages = 1,					\
+		.enable_reg = enreg,					\
+		.enable_mask = BIT(enbit),				\
+		.min_uV = volt,						\
+	},								\
+	.qi = BIT(15),							\
+	.modeset_reg = _modeset_reg,					\
+	.modeset_mask = _modeset_mask,					\
+}
+
+static const struct regulator_linear_range buck_volt_range1[] = {
+	REGULATOR_LINEAR_RANGE(700000, 0, 0x7f, 6250),
+};
+
+static const struct regulator_linear_range buck_volt_range2[] = {
+	REGULATOR_LINEAR_RANGE(1400000, 0, 0x7f, 12500),
+};
+
+static const struct regulator_linear_range buck_volt_range3[] = {
+	REGULATOR_LINEAR_RANGE(500000, 0, 0x3f, 50000),
+};
+
+static const u32 ldo_volt_table1[] = {
+	3300000, 3400000, 3500000, 3600000,
+};
+
+static const u32 ldo_volt_table2[] = {
+	1500000, 1800000, 2500000, 2800000,
+};
+
+static const u32 ldo_volt_table3[] = {
+	1800000, 3300000,
+};
+
+static const u32 ldo_volt_table4[] = {
+	3000000, 3300000,
+};
+
+static const u32 ldo_volt_table5[] = {
+	1200000, 1300000, 1500000, 1800000, 2000000, 2800000, 3000000, 3300000,
+};
+
+static const u32 ldo_volt_table6[] = {
+	1200000, 1300000, 1500000, 1800000, 2500000, 2800000, 3000000, 2000000,
+};
+
+static const u32 ldo_volt_table7[] = {
+	1200000, 1300000, 1500000, 1800000,
+};
+
+static const u32 ldo_volt_table8[] = {
+	1800000, 3000000,
+};
+
+static const u32 ldo_volt_table9[] = {
+	1200000, 1350000, 1500000, 1800000,
+};
+
+static const u32 ldo_volt_table10[] = {
+	1200000, 1300000, 1500000, 1800000,
+};
+
+static int mt6323_get_status(struct regulator_dev *rdev)
+{
+	int ret;
+	u32 regval;
+	struct mt6323_regulator_info *info = rdev_get_drvdata(rdev);
+
+	ret = regmap_read(rdev->regmap, info->desc.enable_reg, &regval);
+	if (ret != 0) {
+		dev_err(&rdev->dev, "Failed to get enable reg: %d\n", ret);
+		return ret;
+	}
+
+	return (regval & info->qi) ? REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;
+}
+
+static int mt6323_ldo_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	int ret, val = 0;
+	struct mt6323_regulator_info *info = rdev_get_drvdata(rdev);
+
+	if (!info->modeset_mask) {
+		dev_err(&rdev->dev, "regulator %s doesn't support set_mode\n",
+			info->desc.name);
+		return -EINVAL;
+	}
+
+	switch (mode) {
+	case REGULATOR_MODE_STANDBY:
+		val = MT6323_LDO_MODE_LP;
+		break;
+	case REGULATOR_MODE_NORMAL:
+		val = MT6323_LDO_MODE_NORMAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	val <<= ffs(info->modeset_mask) - 1;
+
+	ret = regmap_update_bits(rdev->regmap, info->modeset_reg,
+				  info->modeset_mask, val);
+
+	return ret;
+}
+
+static unsigned int mt6323_ldo_get_mode(struct regulator_dev *rdev)
+{
+	unsigned int val;
+	unsigned int mode;
+	int ret;
+	struct mt6323_regulator_info *info = rdev_get_drvdata(rdev);
+
+	if (!info->modeset_mask) {
+		dev_err(&rdev->dev, "regulator %s doesn't support get_mode\n",
+			info->desc.name);
+		return -EINVAL;
+	}
+
+	ret = regmap_read(rdev->regmap, info->modeset_reg, &val);
+	if (ret < 0)
+		return ret;
+
+	val &= info->modeset_mask;
+	val >>= ffs(info->modeset_mask) - 1;
+
+	if (val & 0x1)
+		mode = REGULATOR_MODE_STANDBY;
+	else
+		mode = REGULATOR_MODE_NORMAL;
+
+	return mode;
+}
+
+static struct regulator_ops mt6323_volt_range_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+	.map_voltage = regulator_map_voltage_linear_range,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = mt6323_get_status,
+};
+
+static struct regulator_ops mt6323_volt_table_ops = {
+	.list_voltage = regulator_list_voltage_table,
+	.map_voltage = regulator_map_voltage_iterate,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = mt6323_get_status,
+	.set_mode = mt6323_ldo_set_mode,
+	.get_mode = mt6323_ldo_get_mode,
+};
+
+static struct regulator_ops mt6323_volt_fixed_ops = {
+	.list_voltage = regulator_list_voltage_linear,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = mt6323_get_status,
+	.set_mode = mt6323_ldo_set_mode,
+	.get_mode = mt6323_ldo_get_mode,
+};
+
+/* The array is indexed by id(MT6323_ID_XXX) */
+static struct mt6323_regulator_info mt6323_regulators[] = {
+	MT6323_BUCK("buck_vproc", VPROC, 700000, 1493750, 6250,
+		buck_volt_range1, MT6323_VPROC_CON7, MT6323_VPROC_CON9, 0x7f,
+		MT6323_VPROC_CON10, MT6323_VPROC_CON5),
+	MT6323_BUCK("buck_vsys", VSYS, 1400000, 2987500, 12500,
+		buck_volt_range2, MT6323_VSYS_CON7, MT6323_VSYS_CON9, 0x7f,
+		MT6323_VSYS_CON10, MT6323_VSYS_CON5),
+	MT6323_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
+		buck_volt_range3, MT6323_VPA_CON7, MT6323_VPA_CON9,
+		0x3f, MT6323_VPA_CON10, MT6323_VPA_CON5),
+	MT6323_REG_FIXED("ldo_vtcxo", VTCXO, MT6323_ANALDO_CON1, 10, 2800000,
+		MT6323_ANALDO_CON1, 0x2),
+	MT6323_REG_FIXED("ldo_vcn28", VCN28, MT6323_ANALDO_CON19, 12, 2800000,
+		MT6323_ANALDO_CON20, 0x2),
+	MT6323_LDO("ldo_vcn33_bt", VCN33_BT, ldo_volt_table1,
+		MT6323_ANALDO_CON16, 7, MT6323_ANALDO_CON16, 0xC,
+		MT6323_ANALDO_CON21, 0x2),
+	MT6323_LDO("ldo_vcn33_wifi", VCN33_WIFI, ldo_volt_table1,
+		MT6323_ANALDO_CON17, 12, MT6323_ANALDO_CON16, 0xC,
+		MT6323_ANALDO_CON21, 0x2),
+	MT6323_REG_FIXED("ldo_va", VA, MT6323_ANALDO_CON2, 14, 2800000,
+		MT6323_ANALDO_CON2, 0x2),
+	MT6323_LDO("ldo_vcama", VCAMA, ldo_volt_table2,
+		MT6323_ANALDO_CON4, 15, MT6323_ANALDO_CON10, 0x60, -1, 0),
+	MT6323_REG_FIXED("ldo_vio28", VIO28, MT6323_DIGLDO_CON0, 14, 2800000,
+		MT6323_DIGLDO_CON0, 0x2),
+	MT6323_REG_FIXED("ldo_vusb", VUSB, MT6323_DIGLDO_CON2, 14, 3300000,
+		MT6323_DIGLDO_CON2, 0x2),
+	MT6323_LDO("ldo_vmc", VMC, ldo_volt_table3,
+		MT6323_DIGLDO_CON3, 12, MT6323_DIGLDO_CON24, 0x10,
+		MT6323_DIGLDO_CON3, 0x2),
+	MT6323_LDO("ldo_vmch", VMCH, ldo_volt_table4,
+		MT6323_DIGLDO_CON5, 14, MT6323_DIGLDO_CON26, 0x80,
+		MT6323_DIGLDO_CON5, 0x2),
+	MT6323_LDO("ldo_vemc3v3", VEMC3V3, ldo_volt_table4,
+		MT6323_DIGLDO_CON6, 14, MT6323_DIGLDO_CON27, 0x80,
+		MT6323_DIGLDO_CON6, 0x2),
+	MT6323_LDO("ldo_vgp1", VGP1, ldo_volt_table5,
+		MT6323_DIGLDO_CON7, 15, MT6323_DIGLDO_CON28, 0xE0,
+		MT6323_DIGLDO_CON7, 0x2),
+	MT6323_LDO("ldo_vgp2", VGP2, ldo_volt_table6,
+		MT6323_DIGLDO_CON8, 15, MT6323_DIGLDO_CON29, 0xE0,
+		MT6323_DIGLDO_CON8, 0x2),
+	MT6323_LDO("ldo_vgp3", VGP3, ldo_volt_table7,
+		MT6323_DIGLDO_CON9, 15, MT6323_DIGLDO_CON30, 0x60,
+		MT6323_DIGLDO_CON9, 0x2),
+	MT6323_REG_FIXED("ldo_vcn18", VCN18, MT6323_DIGLDO_CON11, 14, 1800000,
+		MT6323_DIGLDO_CON11, 0x2),
+	MT6323_LDO("ldo_vsim1", VSIM1, ldo_volt_table8,
+		MT6323_DIGLDO_CON13, 15, MT6323_DIGLDO_CON34, 0x20,
+		MT6323_DIGLDO_CON13, 0x2),
+	MT6323_LDO("ldo_vsim2", VSIM2, ldo_volt_table8,
+		MT6323_DIGLDO_CON14, 15, MT6323_DIGLDO_CON35, 0x20,
+		MT6323_DIGLDO_CON14, 0x2),
+	MT6323_REG_FIXED("ldo_vrtc", VRTC, MT6323_DIGLDO_CON15, 8, 2800000,
+		-1, 0),
+	MT6323_LDO("ldo_vcamaf", VCAMAF, ldo_volt_table5,
+		MT6323_DIGLDO_CON31, 15, MT6323_DIGLDO_CON32, 0xE0,
+		MT6323_DIGLDO_CON31, 0x2),
+	MT6323_LDO("ldo_vibr", VIBR, ldo_volt_table5,
+		MT6323_DIGLDO_CON39, 15, MT6323_DIGLDO_CON40, 0xE0,
+		MT6323_DIGLDO_CON39, 0x2),
+	MT6323_REG_FIXED("ldo_vrf18", VRF18, MT6323_DIGLDO_CON45, 15, 1825000,
+		MT6323_DIGLDO_CON45, 0x2),
+	MT6323_LDO("ldo_vm", VM, ldo_volt_table9,
+		MT6323_DIGLDO_CON47, 14, MT6323_DIGLDO_CON48, 0x30,
+		MT6323_DIGLDO_CON47, 0x2),
+	MT6323_REG_FIXED("ldo_vio18", VIO18, MT6323_DIGLDO_CON49, 14, 1800000,
+		MT6323_DIGLDO_CON49, 0x2),
+	MT6323_LDO("ldo_vcamd", VCAMD, ldo_volt_table10,
+		MT6323_DIGLDO_CON51, 14, MT6323_DIGLDO_CON52, 0x60,
+		MT6323_DIGLDO_CON51, 0x2),
+	MT6323_REG_FIXED("ldo_vcamio", VCAMIO, MT6323_DIGLDO_CON53, 14, 1800000,
+		MT6323_DIGLDO_CON53, 0x2),
+};
+
+static int mt6323_set_buck_vosel_reg(struct platform_device *pdev)
+{
+	struct mt6397_chip *mt6323 = dev_get_drvdata(pdev->dev.parent);
+	int i;
+	u32 regval;
+
+	for (i = 0; i < MT6323_MAX_REGULATOR; i++) {
+		if (mt6323_regulators[i].vselctrl_reg) {
+			if (regmap_read(mt6323->regmap,
+				mt6323_regulators[i].vselctrl_reg,
+				&regval) < 0) {
+				dev_err(&pdev->dev,
+					"Failed to read buck ctrl\n");
+				return -EIO;
+			}
+
+			if (regval & mt6323_regulators[i].vselctrl_mask) {
+				mt6323_regulators[i].desc.vsel_reg =
+				mt6323_regulators[i].vselon_reg;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int mt6323_regulator_probe(struct platform_device *pdev)
+{
+	struct mt6397_chip *mt6323 = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_config config = {};
+	struct regulator_dev *rdev;
+	struct regulation_constraints *c;
+	int i;
+	u32 reg_value;
+
+	/* Query buck controller to select activated voltage register part */
+	if (mt6323_set_buck_vosel_reg(pdev))
+		return -EIO;
+
+	/* Read PMIC chip revision to update constraints and voltage table */
+	if (regmap_read(mt6323->regmap, MT6323_CID, &reg_value) < 0) {
+		dev_err(&pdev->dev, "Failed to read Chip ID\n");
+		return -EIO;
+	}
+	dev_info(&pdev->dev, "Chip ID = 0x%x\n", reg_value);
+
+	for (i = 0; i < MT6323_MAX_REGULATOR; i++) {
+		config.dev = &pdev->dev;
+		config.driver_data = &mt6323_regulators[i];
+		config.regmap = mt6323->regmap;
+		rdev = devm_regulator_register(&pdev->dev,
+				&mt6323_regulators[i].desc, &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "failed to register %s\n",
+				mt6323_regulators[i].desc.name);
+			return PTR_ERR(rdev);
+		}
+
+		/* Constrain board-specific capabilities according to what
+		 * this driver and the chip itself can actually do.
+		 */
+		c = rdev->constraints;
+		c->valid_modes_mask |= REGULATOR_MODE_NORMAL |
+				       REGULATOR_MODE_STANDBY;
+		c->valid_ops_mask |= REGULATOR_CHANGE_MODE;
+	}
+	return 0;
+}
+
+static struct platform_driver mt6323_regulator_driver = {
+	.driver = {
+		.name = "mt6323-regulator",
+	},
+	.probe = mt6323_regulator_probe,
+};
+
+module_platform_driver(mt6323_regulator_driver);
+
+MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
+MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mt6323-regulator");
diff --git a/include/linux/regulator/mt6323-regulator.h b/include/linux/regulator/mt6323-regulator.h
new file mode 100644
index 0000000..67011cd
--- /dev/null
+++ b/include/linux/regulator/mt6323-regulator.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2016 MediaTek Inc.
+ * Author: Chen Zhong <chen.zhong@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_REGULATOR_MT6323_H
+#define __LINUX_REGULATOR_MT6323_H
+
+enum {
+	MT6323_ID_VPROC = 0,
+	MT6323_ID_VSYS,
+	MT6323_ID_VPA,
+	MT6323_ID_VTCXO,
+	MT6323_ID_VCN28,
+	MT6323_ID_VCN33_BT,
+	MT6323_ID_VCN33_WIFI,
+	MT6323_ID_VA,
+	MT6323_ID_VCAMA,
+	MT6323_ID_VIO28 = 9,
+	MT6323_ID_VUSB,
+	MT6323_ID_VMC,
+	MT6323_ID_VMCH,
+	MT6323_ID_VEMC3V3,
+	MT6323_ID_VGP1,
+	MT6323_ID_VGP2,
+	MT6323_ID_VGP3,
+	MT6323_ID_VCN18,
+	MT6323_ID_VSIM1,
+	MT6323_ID_VSIM2,
+	MT6323_ID_VRTC,
+	MT6323_ID_VCAMAF,
+	MT6323_ID_VIBR,
+	MT6323_ID_VRF18,
+	MT6323_ID_VM,
+	MT6323_ID_VIO18,
+	MT6323_ID_VCAMD,
+	MT6323_ID_VCAMIO,
+	MT6323_ID_RG_MAX,
+};
+
+#define MT6323_MAX_REGULATOR	MT6323_ID_RG_MAX
+
+#endif /* __LINUX_REGULATOR_MT6323_H */
-- 
1.7.10.4

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

From: Chen Zhong <chen.zhong@mediatek.com>

The MT6323 is a regulator found on boards based on MediaTek MT7623 and
probably other SoCs. It is a so called pmic and connects as a slave to
SoC using SPI, wrapped inside the pmic-wrapper.

Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
Signed-off-by: John Crispin <blogic@openwrt.org>
---
 drivers/regulator/Kconfig                  |    9 +
 drivers/regulator/Makefile                 |    1 +
 drivers/regulator/mt6323-regulator.c       |  433 ++++++++++++++++++++++++++++
 include/linux/regulator/mt6323-regulator.h |   52 ++++
 4 files changed, 495 insertions(+)
 create mode 100644 drivers/regulator/mt6323-regulator.c
 create mode 100644 include/linux/regulator/mt6323-regulator.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 8155e80..5e49568 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -462,6 +462,15 @@ config REGULATOR_MT6311
 	  This driver supports the control of different power rails of device
 	  through regulator interface.
 
+config REGULATOR_MT6323
+	tristate "MediaTek MT6323 PMIC"
+	depends on MFD_MT6397
+	help
+	  Say y here to select this option to enable the power regulator of
+	  MediaTek MT6323 PMIC.
+	  This driver supports the control of different power rails of device
+	  through regulator interface.
+
 config REGULATOR_MT6397
 	tristate "MediaTek MT6397 PMIC"
 	depends on MFD_MT6397
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 980b194..8fb55640 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
 obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
 obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
 obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o
+obj-$(CONFIG_REGULATOR_MT6323)	+= mt6323-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
diff --git a/drivers/regulator/mt6323-regulator.c b/drivers/regulator/mt6323-regulator.c
new file mode 100644
index 0000000..1d2621e
--- /dev/null
+++ b/drivers/regulator/mt6323-regulator.c
@@ -0,0 +1,433 @@
+/*
+ * Copyright (c) 2016 MediaTek Inc.
+ * Author: Chen Zhong <chen.zhong@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/mt6397/core.h>
+#include <linux/mfd/mt6323/registers.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/mt6323-regulator.h>
+#include <linux/regulator/of_regulator.h>
+
+#define MT6323_LDO_MODE_NORMAL	0
+#define MT6323_LDO_MODE_LP	1
+
+/*
+ * MT6323 regulators' information
+ *
+ * @desc: standard fields of regulator description.
+ * @qi: Mask for query enable signal status of regulators
+ * @vselon_reg: Register sections for hardware control mode of bucks
+ * @vselctrl_reg: Register for controlling the buck control mode.
+ * @vselctrl_mask: Mask for query buck's voltage control mode.
+ */
+struct mt6323_regulator_info {
+	struct regulator_desc desc;
+	u32 qi;
+	u32 vselon_reg;
+	u32 vselctrl_reg;
+	u32 vselctrl_mask;
+	u32 modeset_reg;
+	u32 modeset_mask;
+};
+
+#define MT6323_BUCK(match, vreg, min, max, step, volt_ranges, enreg,	\
+		vosel, vosel_mask, voselon, vosel_ctrl)			\
+[MT6323_ID_##vreg] = {							\
+	.desc = {							\
+		.name = #vreg,						\
+		.of_match = of_match_ptr(match),			\
+		.ops = &mt6323_volt_range_ops,				\
+		.type = REGULATOR_VOLTAGE,				\
+		.id = MT6323_ID_##vreg,					\
+		.owner = THIS_MODULE,					\
+		.n_voltages = (max - min)/step + 1,			\
+		.linear_ranges = volt_ranges,				\
+		.n_linear_ranges = ARRAY_SIZE(volt_ranges),		\
+		.vsel_reg = vosel,					\
+		.vsel_mask = vosel_mask,				\
+		.enable_reg = enreg,					\
+		.enable_mask = BIT(0),					\
+	},								\
+	.qi = BIT(13),							\
+	.vselon_reg = voselon,						\
+	.vselctrl_reg = vosel_ctrl,					\
+	.vselctrl_mask = BIT(1),					\
+}
+
+#define MT6323_LDO(match, vreg, ldo_volt_table, enreg, enbit, vosel,	\
+		vosel_mask, _modeset_reg, _modeset_mask)		\
+[MT6323_ID_##vreg] = {							\
+	.desc = {							\
+		.name = #vreg,						\
+		.of_match = of_match_ptr(match),			\
+		.ops = &mt6323_volt_table_ops,				\
+		.type = REGULATOR_VOLTAGE,				\
+		.id = MT6323_ID_##vreg,					\
+		.owner = THIS_MODULE,					\
+		.n_voltages = ARRAY_SIZE(ldo_volt_table),		\
+		.volt_table = ldo_volt_table,				\
+		.vsel_reg = vosel,					\
+		.vsel_mask = vosel_mask,				\
+		.enable_reg = enreg,					\
+		.enable_mask = BIT(enbit),				\
+	},								\
+	.qi = BIT(15),							\
+	.modeset_reg = _modeset_reg,					\
+	.modeset_mask = _modeset_mask,					\
+}
+
+#define MT6323_REG_FIXED(match, vreg, enreg, enbit, volt,		\
+		_modeset_reg, _modeset_mask)				\
+[MT6323_ID_##vreg] = {							\
+	.desc = {							\
+		.name = #vreg,						\
+		.of_match = of_match_ptr(match),			\
+		.ops = &mt6323_volt_fixed_ops,				\
+		.type = REGULATOR_VOLTAGE,				\
+		.id = MT6323_ID_##vreg,					\
+		.owner = THIS_MODULE,					\
+		.n_voltages = 1,					\
+		.enable_reg = enreg,					\
+		.enable_mask = BIT(enbit),				\
+		.min_uV = volt,						\
+	},								\
+	.qi = BIT(15),							\
+	.modeset_reg = _modeset_reg,					\
+	.modeset_mask = _modeset_mask,					\
+}
+
+static const struct regulator_linear_range buck_volt_range1[] = {
+	REGULATOR_LINEAR_RANGE(700000, 0, 0x7f, 6250),
+};
+
+static const struct regulator_linear_range buck_volt_range2[] = {
+	REGULATOR_LINEAR_RANGE(1400000, 0, 0x7f, 12500),
+};
+
+static const struct regulator_linear_range buck_volt_range3[] = {
+	REGULATOR_LINEAR_RANGE(500000, 0, 0x3f, 50000),
+};
+
+static const u32 ldo_volt_table1[] = {
+	3300000, 3400000, 3500000, 3600000,
+};
+
+static const u32 ldo_volt_table2[] = {
+	1500000, 1800000, 2500000, 2800000,
+};
+
+static const u32 ldo_volt_table3[] = {
+	1800000, 3300000,
+};
+
+static const u32 ldo_volt_table4[] = {
+	3000000, 3300000,
+};
+
+static const u32 ldo_volt_table5[] = {
+	1200000, 1300000, 1500000, 1800000, 2000000, 2800000, 3000000, 3300000,
+};
+
+static const u32 ldo_volt_table6[] = {
+	1200000, 1300000, 1500000, 1800000, 2500000, 2800000, 3000000, 2000000,
+};
+
+static const u32 ldo_volt_table7[] = {
+	1200000, 1300000, 1500000, 1800000,
+};
+
+static const u32 ldo_volt_table8[] = {
+	1800000, 3000000,
+};
+
+static const u32 ldo_volt_table9[] = {
+	1200000, 1350000, 1500000, 1800000,
+};
+
+static const u32 ldo_volt_table10[] = {
+	1200000, 1300000, 1500000, 1800000,
+};
+
+static int mt6323_get_status(struct regulator_dev *rdev)
+{
+	int ret;
+	u32 regval;
+	struct mt6323_regulator_info *info = rdev_get_drvdata(rdev);
+
+	ret = regmap_read(rdev->regmap, info->desc.enable_reg, &regval);
+	if (ret != 0) {
+		dev_err(&rdev->dev, "Failed to get enable reg: %d\n", ret);
+		return ret;
+	}
+
+	return (regval & info->qi) ? REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;
+}
+
+static int mt6323_ldo_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	int ret, val = 0;
+	struct mt6323_regulator_info *info = rdev_get_drvdata(rdev);
+
+	if (!info->modeset_mask) {
+		dev_err(&rdev->dev, "regulator %s doesn't support set_mode\n",
+			info->desc.name);
+		return -EINVAL;
+	}
+
+	switch (mode) {
+	case REGULATOR_MODE_STANDBY:
+		val = MT6323_LDO_MODE_LP;
+		break;
+	case REGULATOR_MODE_NORMAL:
+		val = MT6323_LDO_MODE_NORMAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	val <<= ffs(info->modeset_mask) - 1;
+
+	ret = regmap_update_bits(rdev->regmap, info->modeset_reg,
+				  info->modeset_mask, val);
+
+	return ret;
+}
+
+static unsigned int mt6323_ldo_get_mode(struct regulator_dev *rdev)
+{
+	unsigned int val;
+	unsigned int mode;
+	int ret;
+	struct mt6323_regulator_info *info = rdev_get_drvdata(rdev);
+
+	if (!info->modeset_mask) {
+		dev_err(&rdev->dev, "regulator %s doesn't support get_mode\n",
+			info->desc.name);
+		return -EINVAL;
+	}
+
+	ret = regmap_read(rdev->regmap, info->modeset_reg, &val);
+	if (ret < 0)
+		return ret;
+
+	val &= info->modeset_mask;
+	val >>= ffs(info->modeset_mask) - 1;
+
+	if (val & 0x1)
+		mode = REGULATOR_MODE_STANDBY;
+	else
+		mode = REGULATOR_MODE_NORMAL;
+
+	return mode;
+}
+
+static struct regulator_ops mt6323_volt_range_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+	.map_voltage = regulator_map_voltage_linear_range,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = mt6323_get_status,
+};
+
+static struct regulator_ops mt6323_volt_table_ops = {
+	.list_voltage = regulator_list_voltage_table,
+	.map_voltage = regulator_map_voltage_iterate,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = mt6323_get_status,
+	.set_mode = mt6323_ldo_set_mode,
+	.get_mode = mt6323_ldo_get_mode,
+};
+
+static struct regulator_ops mt6323_volt_fixed_ops = {
+	.list_voltage = regulator_list_voltage_linear,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = mt6323_get_status,
+	.set_mode = mt6323_ldo_set_mode,
+	.get_mode = mt6323_ldo_get_mode,
+};
+
+/* The array is indexed by id(MT6323_ID_XXX) */
+static struct mt6323_regulator_info mt6323_regulators[] = {
+	MT6323_BUCK("buck_vproc", VPROC, 700000, 1493750, 6250,
+		buck_volt_range1, MT6323_VPROC_CON7, MT6323_VPROC_CON9, 0x7f,
+		MT6323_VPROC_CON10, MT6323_VPROC_CON5),
+	MT6323_BUCK("buck_vsys", VSYS, 1400000, 2987500, 12500,
+		buck_volt_range2, MT6323_VSYS_CON7, MT6323_VSYS_CON9, 0x7f,
+		MT6323_VSYS_CON10, MT6323_VSYS_CON5),
+	MT6323_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
+		buck_volt_range3, MT6323_VPA_CON7, MT6323_VPA_CON9,
+		0x3f, MT6323_VPA_CON10, MT6323_VPA_CON5),
+	MT6323_REG_FIXED("ldo_vtcxo", VTCXO, MT6323_ANALDO_CON1, 10, 2800000,
+		MT6323_ANALDO_CON1, 0x2),
+	MT6323_REG_FIXED("ldo_vcn28", VCN28, MT6323_ANALDO_CON19, 12, 2800000,
+		MT6323_ANALDO_CON20, 0x2),
+	MT6323_LDO("ldo_vcn33_bt", VCN33_BT, ldo_volt_table1,
+		MT6323_ANALDO_CON16, 7, MT6323_ANALDO_CON16, 0xC,
+		MT6323_ANALDO_CON21, 0x2),
+	MT6323_LDO("ldo_vcn33_wifi", VCN33_WIFI, ldo_volt_table1,
+		MT6323_ANALDO_CON17, 12, MT6323_ANALDO_CON16, 0xC,
+		MT6323_ANALDO_CON21, 0x2),
+	MT6323_REG_FIXED("ldo_va", VA, MT6323_ANALDO_CON2, 14, 2800000,
+		MT6323_ANALDO_CON2, 0x2),
+	MT6323_LDO("ldo_vcama", VCAMA, ldo_volt_table2,
+		MT6323_ANALDO_CON4, 15, MT6323_ANALDO_CON10, 0x60, -1, 0),
+	MT6323_REG_FIXED("ldo_vio28", VIO28, MT6323_DIGLDO_CON0, 14, 2800000,
+		MT6323_DIGLDO_CON0, 0x2),
+	MT6323_REG_FIXED("ldo_vusb", VUSB, MT6323_DIGLDO_CON2, 14, 3300000,
+		MT6323_DIGLDO_CON2, 0x2),
+	MT6323_LDO("ldo_vmc", VMC, ldo_volt_table3,
+		MT6323_DIGLDO_CON3, 12, MT6323_DIGLDO_CON24, 0x10,
+		MT6323_DIGLDO_CON3, 0x2),
+	MT6323_LDO("ldo_vmch", VMCH, ldo_volt_table4,
+		MT6323_DIGLDO_CON5, 14, MT6323_DIGLDO_CON26, 0x80,
+		MT6323_DIGLDO_CON5, 0x2),
+	MT6323_LDO("ldo_vemc3v3", VEMC3V3, ldo_volt_table4,
+		MT6323_DIGLDO_CON6, 14, MT6323_DIGLDO_CON27, 0x80,
+		MT6323_DIGLDO_CON6, 0x2),
+	MT6323_LDO("ldo_vgp1", VGP1, ldo_volt_table5,
+		MT6323_DIGLDO_CON7, 15, MT6323_DIGLDO_CON28, 0xE0,
+		MT6323_DIGLDO_CON7, 0x2),
+	MT6323_LDO("ldo_vgp2", VGP2, ldo_volt_table6,
+		MT6323_DIGLDO_CON8, 15, MT6323_DIGLDO_CON29, 0xE0,
+		MT6323_DIGLDO_CON8, 0x2),
+	MT6323_LDO("ldo_vgp3", VGP3, ldo_volt_table7,
+		MT6323_DIGLDO_CON9, 15, MT6323_DIGLDO_CON30, 0x60,
+		MT6323_DIGLDO_CON9, 0x2),
+	MT6323_REG_FIXED("ldo_vcn18", VCN18, MT6323_DIGLDO_CON11, 14, 1800000,
+		MT6323_DIGLDO_CON11, 0x2),
+	MT6323_LDO("ldo_vsim1", VSIM1, ldo_volt_table8,
+		MT6323_DIGLDO_CON13, 15, MT6323_DIGLDO_CON34, 0x20,
+		MT6323_DIGLDO_CON13, 0x2),
+	MT6323_LDO("ldo_vsim2", VSIM2, ldo_volt_table8,
+		MT6323_DIGLDO_CON14, 15, MT6323_DIGLDO_CON35, 0x20,
+		MT6323_DIGLDO_CON14, 0x2),
+	MT6323_REG_FIXED("ldo_vrtc", VRTC, MT6323_DIGLDO_CON15, 8, 2800000,
+		-1, 0),
+	MT6323_LDO("ldo_vcamaf", VCAMAF, ldo_volt_table5,
+		MT6323_DIGLDO_CON31, 15, MT6323_DIGLDO_CON32, 0xE0,
+		MT6323_DIGLDO_CON31, 0x2),
+	MT6323_LDO("ldo_vibr", VIBR, ldo_volt_table5,
+		MT6323_DIGLDO_CON39, 15, MT6323_DIGLDO_CON40, 0xE0,
+		MT6323_DIGLDO_CON39, 0x2),
+	MT6323_REG_FIXED("ldo_vrf18", VRF18, MT6323_DIGLDO_CON45, 15, 1825000,
+		MT6323_DIGLDO_CON45, 0x2),
+	MT6323_LDO("ldo_vm", VM, ldo_volt_table9,
+		MT6323_DIGLDO_CON47, 14, MT6323_DIGLDO_CON48, 0x30,
+		MT6323_DIGLDO_CON47, 0x2),
+	MT6323_REG_FIXED("ldo_vio18", VIO18, MT6323_DIGLDO_CON49, 14, 1800000,
+		MT6323_DIGLDO_CON49, 0x2),
+	MT6323_LDO("ldo_vcamd", VCAMD, ldo_volt_table10,
+		MT6323_DIGLDO_CON51, 14, MT6323_DIGLDO_CON52, 0x60,
+		MT6323_DIGLDO_CON51, 0x2),
+	MT6323_REG_FIXED("ldo_vcamio", VCAMIO, MT6323_DIGLDO_CON53, 14, 1800000,
+		MT6323_DIGLDO_CON53, 0x2),
+};
+
+static int mt6323_set_buck_vosel_reg(struct platform_device *pdev)
+{
+	struct mt6397_chip *mt6323 = dev_get_drvdata(pdev->dev.parent);
+	int i;
+	u32 regval;
+
+	for (i = 0; i < MT6323_MAX_REGULATOR; i++) {
+		if (mt6323_regulators[i].vselctrl_reg) {
+			if (regmap_read(mt6323->regmap,
+				mt6323_regulators[i].vselctrl_reg,
+				&regval) < 0) {
+				dev_err(&pdev->dev,
+					"Failed to read buck ctrl\n");
+				return -EIO;
+			}
+
+			if (regval & mt6323_regulators[i].vselctrl_mask) {
+				mt6323_regulators[i].desc.vsel_reg =
+				mt6323_regulators[i].vselon_reg;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int mt6323_regulator_probe(struct platform_device *pdev)
+{
+	struct mt6397_chip *mt6323 = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_config config = {};
+	struct regulator_dev *rdev;
+	struct regulation_constraints *c;
+	int i;
+	u32 reg_value;
+
+	/* Query buck controller to select activated voltage register part */
+	if (mt6323_set_buck_vosel_reg(pdev))
+		return -EIO;
+
+	/* Read PMIC chip revision to update constraints and voltage table */
+	if (regmap_read(mt6323->regmap, MT6323_CID, &reg_value) < 0) {
+		dev_err(&pdev->dev, "Failed to read Chip ID\n");
+		return -EIO;
+	}
+	dev_info(&pdev->dev, "Chip ID = 0x%x\n", reg_value);
+
+	for (i = 0; i < MT6323_MAX_REGULATOR; i++) {
+		config.dev = &pdev->dev;
+		config.driver_data = &mt6323_regulators[i];
+		config.regmap = mt6323->regmap;
+		rdev = devm_regulator_register(&pdev->dev,
+				&mt6323_regulators[i].desc, &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "failed to register %s\n",
+				mt6323_regulators[i].desc.name);
+			return PTR_ERR(rdev);
+		}
+
+		/* Constrain board-specific capabilities according to what
+		 * this driver and the chip itself can actually do.
+		 */
+		c = rdev->constraints;
+		c->valid_modes_mask |= REGULATOR_MODE_NORMAL |
+				       REGULATOR_MODE_STANDBY;
+		c->valid_ops_mask |= REGULATOR_CHANGE_MODE;
+	}
+	return 0;
+}
+
+static struct platform_driver mt6323_regulator_driver = {
+	.driver = {
+		.name = "mt6323-regulator",
+	},
+	.probe = mt6323_regulator_probe,
+};
+
+module_platform_driver(mt6323_regulator_driver);
+
+MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
+MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mt6323-regulator");
diff --git a/include/linux/regulator/mt6323-regulator.h b/include/linux/regulator/mt6323-regulator.h
new file mode 100644
index 0000000..67011cd
--- /dev/null
+++ b/include/linux/regulator/mt6323-regulator.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2016 MediaTek Inc.
+ * Author: Chen Zhong <chen.zhong@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_REGULATOR_MT6323_H
+#define __LINUX_REGULATOR_MT6323_H
+
+enum {
+	MT6323_ID_VPROC = 0,
+	MT6323_ID_VSYS,
+	MT6323_ID_VPA,
+	MT6323_ID_VTCXO,
+	MT6323_ID_VCN28,
+	MT6323_ID_VCN33_BT,
+	MT6323_ID_VCN33_WIFI,
+	MT6323_ID_VA,
+	MT6323_ID_VCAMA,
+	MT6323_ID_VIO28 = 9,
+	MT6323_ID_VUSB,
+	MT6323_ID_VMC,
+	MT6323_ID_VMCH,
+	MT6323_ID_VEMC3V3,
+	MT6323_ID_VGP1,
+	MT6323_ID_VGP2,
+	MT6323_ID_VGP3,
+	MT6323_ID_VCN18,
+	MT6323_ID_VSIM1,
+	MT6323_ID_VSIM2,
+	MT6323_ID_VRTC,
+	MT6323_ID_VCAMAF,
+	MT6323_ID_VIBR,
+	MT6323_ID_VRF18,
+	MT6323_ID_VM,
+	MT6323_ID_VIO18,
+	MT6323_ID_VCAMD,
+	MT6323_ID_VCAMIO,
+	MT6323_ID_RG_MAX,
+};
+
+#define MT6323_MAX_REGULATOR	MT6323_ID_RG_MAX
+
+#endif /* __LINUX_REGULATOR_MT6323_H */
-- 
1.7.10.4

Re: [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator

[Mark Brown]

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

On Mon, Jan 25, 2016 at 11:40:04AM +0100, John Crispin wrote:
> Based on the MT6397 binding documentation.
> 
> Signed-off-by: John Crispin <blogic@openwrt.org>
> Cc: devicetree@vger.kernel.org

In reply to your previous submission I said:

| important.  Please also use subject lines matching the style for the
| subsystem (for patch 1).

Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed.  Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing.  If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.

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

Re: [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator

[Mark Brown]

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

On Mon, Jan 25, 2016 at 11:40:04AM +0100, John Crispin wrote:
> Based on the MT6397 binding documentation.
> 
> Signed-off-by: John Crispin <blogic-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

In reply to your previous submission I said:

| important.  Please also use subject lines matching the style for the
| subsystem (for patch 1).

Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed.  Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing.  If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.

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

[PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator

[Mark Brown]

On Mon, Jan 25, 2016 at 11:40:04AM +0100, John Crispin wrote:
> Based on the MT6397 binding documentation.
> 
> Signed-off-by: John Crispin <blogic@openwrt.org>
> Cc: devicetree at vger.kernel.org

In reply to your previous submission I said:

| important.  Please also use subject lines matching the style for the
| subsystem (for patch 1).

Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed.  Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing.  If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.
-------------- 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/20160125/d1457ad8/attachment.sig>

Re: [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator

[John Crispin]

On 25/01/2016 12:37, Mark Brown wrote:
> On Mon, Jan 25, 2016 at 11:40:04AM +0100, John Crispin wrote:
>> Based on the MT6397 binding documentation.
>>
>> Signed-off-by: John Crispin <blogic@openwrt.org>
>> Cc: devicetree@vger.kernel.org
> 
> In reply to your previous submission I said:
> 
> | important.  Please also use subject lines matching the style for the
> | subsystem (for patch 1).
> 
> Please don't ignore review comments, people are generally making them
> for a reason and are likely to have the same concerns if issues remain
> unaddressed.  Having to repeat the same comments can get repetitive and
> make people question the value of time spent reviewing.  If you disagree
> with the review comments that's fine but you need to reply and discuss
> your concerns so that the reviewer can understand your decisions.
> 

Hi,

sorry about that. I've been juggling with a pile of patches the last
couple of weeks and missed folding the fixup into the patch before
sending it. I've just sent you a V3.

	John

Re: [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator

[John Crispin]

On 25/01/2016 12:37, Mark Brown wrote:
> On Mon, Jan 25, 2016 at 11:40:04AM +0100, John Crispin wrote:
>> Based on the MT6397 binding documentation.
>>
>> Signed-off-by: John Crispin <blogic-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> 
> In reply to your previous submission I said:
> 
> | important.  Please also use subject lines matching the style for the
> | subsystem (for patch 1).
> 
> Please don't ignore review comments, people are generally making them
> for a reason and are likely to have the same concerns if issues remain
> unaddressed.  Having to repeat the same comments can get repetitive and
> make people question the value of time spent reviewing.  If you disagree
> with the review comments that's fine but you need to reply and discuss
> your concerns so that the reviewer can understand your decisions.
> 

Hi,

sorry about that. I've been juggling with a pile of patches the last
couple of weeks and missed folding the fixup into the patch before
sending it. I've just sent you a V3.

	John
--
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 V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator

[John Crispin]

On 25/01/2016 12:37, Mark Brown wrote:
> On Mon, Jan 25, 2016 at 11:40:04AM +0100, John Crispin wrote:
>> Based on the MT6397 binding documentation.
>>
>> Signed-off-by: John Crispin <blogic@openwrt.org>
>> Cc: devicetree at vger.kernel.org
> 
> In reply to your previous submission I said:
> 
> | important.  Please also use subject lines matching the style for the
> | subsystem (for patch 1).
> 
> Please don't ignore review comments, people are generally making them
> for a reason and are likely to have the same concerns if issues remain
> unaddressed.  Having to repeat the same comments can get repetitive and
> make people question the value of time spent reviewing.  If you disagree
> with the review comments that's fine but you need to reply and discuss
> your concerns so that the reviewer can understand your decisions.
> 

Hi,

sorry about that. I've been juggling with a pile of patches the last
couple of weeks and missed folding the fixup into the patch before
sending it. I've just sent you a V3.

	John

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello,

On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic@openwrt.org> wrote:
> From: Chen Zhong <chen.zhong@mediatek.com>

[snip]

> +}
> +
> +static struct platform_driver mt6323_regulator_driver = {
> +       .driver = {
> +               .name = "mt6323-regulator",
> +       },
> +       .probe = mt6323_regulator_probe,
> +};
> +

You don't have a .of_match table but according the DT bindings, the
compatible string "mediatek,mt6323-regulator" should be used so there
should be a OF match table or the vendor prefix of the compatible
string won't be used for matching (i.e: fallbacks to the driver .name
for match).

> +module_platform_driver(mt6323_regulator_driver);
> +
> +MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:mt6323-regulator");

This alias should not be needed if you provide a OF match table and a
MODULE_DEVICE_TABLE(of, foo);

Best regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello,

On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic@openwrt.org> wrote:
> From: Chen Zhong <chen.zhong@mediatek.com>

[snip]

> +}
> +
> +static struct platform_driver mt6323_regulator_driver = {
> +       .driver = {
> +               .name = "mt6323-regulator",
> +       },
> +       .probe = mt6323_regulator_probe,
> +};
> +

You don't have a .of_match table but according the DT bindings, the
compatible string "mediatek,mt6323-regulator" should be used so there
should be a OF match table or the vendor prefix of the compatible
string won't be used for matching (i.e: fallbacks to the driver .name
for match).

> +module_platform_driver(mt6323_regulator_driver);
> +
> +MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:mt6323-regulator");

This alias should not be needed if you provide a OF match table and a
MODULE_DEVICE_TABLE(of, foo);

Best regards,
Javier

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello,

On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic@openwrt.org> wrote:
> From: Chen Zhong <chen.zhong@mediatek.com>

[snip]

> +}
> +
> +static struct platform_driver mt6323_regulator_driver = {
> +       .driver = {
> +               .name = "mt6323-regulator",
> +       },
> +       .probe = mt6323_regulator_probe,
> +};
> +

You don't have a .of_match table but according the DT bindings, the
compatible string "mediatek,mt6323-regulator" should be used so there
should be a OF match table or the vendor prefix of the compatible
string won't be used for matching (i.e: fallbacks to the driver .name
for match).

> +module_platform_driver(mt6323_regulator_driver);
> +
> +MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:mt6323-regulator");

This alias should not be needed if you provide a OF match table and a
MODULE_DEVICE_TABLE(of, foo);

Best regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

Hi,

On 25/01/2016 13:11, Javier Martinez Canillas wrote:
> Hello,
> 
> On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic@openwrt.org> wrote:
>> From: Chen Zhong <chen.zhong@mediatek.com>
> 
> [snip]
> 
>> +}
>> +
>> +static struct platform_driver mt6323_regulator_driver = {
>> +       .driver = {
>> +               .name = "mt6323-regulator",
>> +       },
>> +       .probe = mt6323_regulator_probe,
>> +};
>> +
> 
> You don't have a .of_match table but according the DT bindings, the
> compatible string "mediatek,mt6323-regulator" should be used so there
> should be a OF match table or the vendor prefix of the compatible
> string won't be used for matching (i.e: fallbacks to the driver .name
> for match).

the driver is probed via drivers/mfd/mt6397-core.c and does not require
the OF match table. It loads fine just like the mt6397 driver.

> 
>> +module_platform_driver(mt6323_regulator_driver);
>> +
>> +MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
>> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:mt6323-regulator");
> 
> This alias should not be needed if you provide a OF match table and a
> MODULE_DEVICE_TABLE(of, foo);

see above.

	John

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

Hi,

On 25/01/2016 13:11, Javier Martinez Canillas wrote:
> Hello,
> 
> On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic@openwrt.org> wrote:
>> From: Chen Zhong <chen.zhong@mediatek.com>
> 
> [snip]
> 
>> +}
>> +
>> +static struct platform_driver mt6323_regulator_driver = {
>> +       .driver = {
>> +               .name = "mt6323-regulator",
>> +       },
>> +       .probe = mt6323_regulator_probe,
>> +};
>> +
> 
> You don't have a .of_match table but according the DT bindings, the
> compatible string "mediatek,mt6323-regulator" should be used so there
> should be a OF match table or the vendor prefix of the compatible
> string won't be used for matching (i.e: fallbacks to the driver .name
> for match).

the driver is probed via drivers/mfd/mt6397-core.c and does not require
the OF match table. It loads fine just like the mt6397 driver.

> 
>> +module_platform_driver(mt6323_regulator_driver);
>> +
>> +MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
>> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:mt6323-regulator");
> 
> This alias should not be needed if you provide a OF match table and a
> MODULE_DEVICE_TABLE(of, foo);

see above.

	John

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

Hi,

On 25/01/2016 13:11, Javier Martinez Canillas wrote:
> Hello,
> 
> On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic@openwrt.org> wrote:
>> From: Chen Zhong <chen.zhong@mediatek.com>
> 
> [snip]
> 
>> +}
>> +
>> +static struct platform_driver mt6323_regulator_driver = {
>> +       .driver = {
>> +               .name = "mt6323-regulator",
>> +       },
>> +       .probe = mt6323_regulator_probe,
>> +};
>> +
> 
> You don't have a .of_match table but according the DT bindings, the
> compatible string "mediatek,mt6323-regulator" should be used so there
> should be a OF match table or the vendor prefix of the compatible
> string won't be used for matching (i.e: fallbacks to the driver .name
> for match).

the driver is probed via drivers/mfd/mt6397-core.c and does not require
the OF match table. It loads fine just like the mt6397 driver.

> 
>> +module_platform_driver(mt6323_regulator_driver);
>> +
>> +MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
>> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:mt6323-regulator");
> 
> This alias should not be needed if you provide a OF match table and a
> MODULE_DEVICE_TABLE(of, foo);

see above.

	John

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello John,

On Mon, Jan 25, 2016 at 9:19 AM, John Crispin <blogic@openwrt.org> wrote:
> Hi,
>
> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
>> Hello,
>>
>> On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic@openwrt.org> wrote:
>>> From: Chen Zhong <chen.zhong@mediatek.com>
>>
>> [snip]
>>
>>> +}
>>> +
>>> +static struct platform_driver mt6323_regulator_driver = {
>>> +       .driver = {
>>> +               .name = "mt6323-regulator",
>>> +       },
>>> +       .probe = mt6323_regulator_probe,
>>> +};
>>> +
>>
>> You don't have a .of_match table but according the DT bindings, the
>> compatible string "mediatek,mt6323-regulator" should be used so there
>> should be a OF match table or the vendor prefix of the compatible
>> string won't be used for matching (i.e: fallbacks to the driver .name
>> for match).
>
> the driver is probed via drivers/mfd/mt6397-core.c and does not require
> the OF match table. It loads fine just like the mt6397 driver.
>

The MFD driver sets .of_compatible = "mediatek,mt6397-regulator" for
the MFD cell so I'm pretty sure that the platform bus .uevent callback
will report a MODALIAS=of:NfooT<NULL>C"mediatek,mt6397-regulator" so
that's what udev is going to pass to kmod but:

kmod load of:NfooT<NULL>C"mediatek,mt6397-regulator"

is not going to succeed since there won't be a kernel module with that alias.

Did you really try building it as a module and checked that it auto loads?

>>
>>> +module_platform_driver(mt6323_regulator_driver);
>>> +
>>> +MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
>>> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_ALIAS("platform:mt6323-regulator");
>>
>> This alias should not be needed if you provide a OF match table and a
>> MODULE_DEVICE_TABLE(of, foo);
>
> see above.
>

In any case, I think the correct thing to do is to provide a .match
and .of_match tables for the platform driver. Since in the future if
you need to support another device with the same driver, you will need
to add another MODULE_ALIAS(). And also is better to be consistent on
what is matched and what is reported to user-space as a module alias.

>         John

Bets regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello John,

On Mon, Jan 25, 2016 at 9:19 AM, John Crispin <blogic@openwrt.org> wrote:
> Hi,
>
> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
>> Hello,
>>
>> On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic@openwrt.org> wrote:
>>> From: Chen Zhong <chen.zhong@mediatek.com>
>>
>> [snip]
>>
>>> +}
>>> +
>>> +static struct platform_driver mt6323_regulator_driver = {
>>> +       .driver = {
>>> +               .name = "mt6323-regulator",
>>> +       },
>>> +       .probe = mt6323_regulator_probe,
>>> +};
>>> +
>>
>> You don't have a .of_match table but according the DT bindings, the
>> compatible string "mediatek,mt6323-regulator" should be used so there
>> should be a OF match table or the vendor prefix of the compatible
>> string won't be used for matching (i.e: fallbacks to the driver .name
>> for match).
>
> the driver is probed via drivers/mfd/mt6397-core.c and does not require
> the OF match table. It loads fine just like the mt6397 driver.
>

The MFD driver sets .of_compatible = "mediatek,mt6397-regulator" for
the MFD cell so I'm pretty sure that the platform bus .uevent callback
will report a MODALIAS=of:NfooT<NULL>C"mediatek,mt6397-regulator" so
that's what udev is going to pass to kmod but:

kmod load of:NfooT<NULL>C"mediatek,mt6397-regulator"

is not going to succeed since there won't be a kernel module with that alias.

Did you really try building it as a module and checked that it auto loads?

>>
>>> +module_platform_driver(mt6323_regulator_driver);
>>> +
>>> +MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
>>> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_ALIAS("platform:mt6323-regulator");
>>
>> This alias should not be needed if you provide a OF match table and a
>> MODULE_DEVICE_TABLE(of, foo);
>
> see above.
>

In any case, I think the correct thing to do is to provide a .match
and .of_match tables for the platform driver. Since in the future if
you need to support another device with the same driver, you will need
to add another MODULE_ALIAS(). And also is better to be consistent on
what is matched and what is reported to user-space as a module alias.

>         John

Bets regards,
Javier

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello John,

On Mon, Jan 25, 2016 at 9:19 AM, John Crispin <blogic@openwrt.org> wrote:
> Hi,
>
> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
>> Hello,
>>
>> On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic@openwrt.org> wrote:
>>> From: Chen Zhong <chen.zhong@mediatek.com>
>>
>> [snip]
>>
>>> +}
>>> +
>>> +static struct platform_driver mt6323_regulator_driver = {
>>> +       .driver = {
>>> +               .name = "mt6323-regulator",
>>> +       },
>>> +       .probe = mt6323_regulator_probe,
>>> +};
>>> +
>>
>> You don't have a .of_match table but according the DT bindings, the
>> compatible string "mediatek,mt6323-regulator" should be used so there
>> should be a OF match table or the vendor prefix of the compatible
>> string won't be used for matching (i.e: fallbacks to the driver .name
>> for match).
>
> the driver is probed via drivers/mfd/mt6397-core.c and does not require
> the OF match table. It loads fine just like the mt6397 driver.
>

The MFD driver sets .of_compatible = "mediatek,mt6397-regulator" for
the MFD cell so I'm pretty sure that the platform bus .uevent callback
will report a MODALIAS=of:NfooT<NULL>C"mediatek,mt6397-regulator" so
that's what udev is going to pass to kmod but:

kmod load of:NfooT<NULL>C"mediatek,mt6397-regulator"

is not going to succeed since there won't be a kernel module with that alias.

Did you really try building it as a module and checked that it auto loads?

>>
>>> +module_platform_driver(mt6323_regulator_driver);
>>> +
>>> +MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
>>> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_ALIAS("platform:mt6323-regulator");
>>
>> This alias should not be needed if you provide a OF match table and a
>> MODULE_DEVICE_TABLE(of, foo);
>
> see above.
>

In any case, I think the correct thing to do is to provide a .match
and .of_match tables for the platform driver. Since in the future if
you need to support another device with the same driver, you will need
to add another MODULE_ALIAS(). And also is better to be consistent on
what is matched and what is reported to user-space as a module alias.

>         John

Bets regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 13:30, Javier Martinez Canillas wrote:
> Hello John,
> 
> On Mon, Jan 25, 2016 at 9:19 AM, John Crispin <blogic-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
>> Hi,
>>
>> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
>>> Hello,
>>>
>>> On Mon, Jan 25, 2016 at 7:40 AM, John Crispin <blogic-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
>>>> From: Chen Zhong <chen.zhong-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>>
>>> [snip]
>>>
>>>> +}
>>>> +
>>>> +static struct platform_driver mt6323_regulator_driver = {
>>>> +       .driver = {
>>>> +               .name = "mt6323-regulator",
>>>> +       },
>>>> +       .probe = mt6323_regulator_probe,
>>>> +};
>>>> +
>>>
>>> You don't have a .of_match table but according the DT bindings, the
>>> compatible string "mediatek,mt6323-regulator" should be used so there
>>> should be a OF match table or the vendor prefix of the compatible
>>> string won't be used for matching (i.e: fallbacks to the driver .name
>>> for match).
>>
>> the driver is probed via drivers/mfd/mt6397-core.c and does not require
>> the OF match table. It loads fine just like the mt6397 driver.
>>
> 
> The MFD driver sets .of_compatible = "mediatek,mt6397-regulator" for
> the MFD cell so I'm pretty sure that the platform bus .uevent callback
> will report a MODALIAS=of:NfooT<NULL>C"mediatek,mt6397-regulator" so
> that's what udev is going to pass to kmod but:
> 
> kmod load of:NfooT<NULL>C"mediatek,mt6397-regulator"
> 
> is not going to succeed since there won't be a kernel module with that alias.
> 
> Did you really try building it as a module and checked that it auto loads?
> 

Hi,

ok, you mean kmod loading. I was only looking at probing. i'll fix it
and send a patch to also fix it for mt6397.

	John

>>>
>>>> +module_platform_driver(mt6323_regulator_driver);
>>>> +
>>>> +MODULE_AUTHOR("Chen Zhong <chen.zhong-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>");
>>>> +MODULE_DESCRIPTION("Regulator Driver for MediaTek MT6397 PMIC");
>>>> +MODULE_LICENSE("GPL");
>>>> +MODULE_ALIAS("platform:mt6323-regulator");
>>>
>>> This alias should not be needed if you provide a OF match table and a
>>> MODULE_DEVICE_TABLE(of, foo);
>>
>> see above.
>>
> 
> In any case, I think the correct thing to do is to provide a .match
> and .of_match tables for the platform driver. Since in the future if
> you need to support another device with the same driver, you will need
> to add another MODULE_ALIAS(). And also is better to be consistent on
> what is matched and what is reported to user-space as a module alias.
> 
>>         John
> 
> Bets regards,
> Javier
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> 

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Mark Brown]

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

On Mon, Jan 25, 2016 at 01:19:46PM +0100, John Crispin wrote:
> On 25/01/2016 13:11, Javier Martinez Canillas wrote:

> > You don't have a .of_match table but according the DT bindings, the
> > compatible string "mediatek,mt6323-regulator" should be used so there
> > should be a OF match table or the vendor prefix of the compatible
> > string won't be used for matching (i.e: fallbacks to the driver .name
> > for match).

> the driver is probed via drivers/mfd/mt6397-core.c and does not require
> the OF match table. It loads fine just like the mt6397 driver.

That's fine but you shouldn't have the compatible string in your binding
document since it's not actually used or needed.

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

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Mark Brown]

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

On Mon, Jan 25, 2016 at 01:19:46PM +0100, John Crispin wrote:
> On 25/01/2016 13:11, Javier Martinez Canillas wrote:

> > You don't have a .of_match table but according the DT bindings, the
> > compatible string "mediatek,mt6323-regulator" should be used so there
> > should be a OF match table or the vendor prefix of the compatible
> > string won't be used for matching (i.e: fallbacks to the driver .name
> > for match).

> the driver is probed via drivers/mfd/mt6397-core.c and does not require
> the OF match table. It loads fine just like the mt6397 driver.

That's fine but you shouldn't have the compatible string in your binding
document since it's not actually used or needed.

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

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Mark Brown]

On Mon, Jan 25, 2016 at 01:19:46PM +0100, John Crispin wrote:
> On 25/01/2016 13:11, Javier Martinez Canillas wrote:

> > You don't have a .of_match table but according the DT bindings, the
> > compatible string "mediatek,mt6323-regulator" should be used so there
> > should be a OF match table or the vendor prefix of the compatible
> > string won't be used for matching (i.e: fallbacks to the driver .name
> > for match).

> the driver is probed via drivers/mfd/mt6397-core.c and does not require
> the OF match table. It loads fine just like the mt6397 driver.

That's fine but you shouldn't have the compatible string in your binding
document since it's not actually used or needed.
-------------- 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/20160125/af9d38a9/attachment-0001.sig>

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 13:35, Mark Brown wrote:
> On Mon, Jan 25, 2016 at 01:19:46PM +0100, John Crispin wrote:
>> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
> 
>>> You don't have a .of_match table but according the DT bindings, the
>>> compatible string "mediatek,mt6323-regulator" should be used so there
>>> should be a OF match table or the vendor prefix of the compatible
>>> string won't be used for matching (i.e: fallbacks to the driver .name
>>> for match).
> 
>> the driver is probed via drivers/mfd/mt6397-core.c and does not require
>> the OF match table. It loads fine just like the mt6397 driver.
> 
> That's fine but you shouldn't have the compatible string in your binding
> document since it's not actually used or needed.
> 
Hi,

correct me if i am wrong but if we remove the compatible string from the
binding document, then we will also have to remove it from the dts and
then the kernel won't be able to match the node to the driver and thus
the regulator phandle derefs will fail.

	John

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 13:35, Mark Brown wrote:
> On Mon, Jan 25, 2016 at 01:19:46PM +0100, John Crispin wrote:
>> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
> 
>>> You don't have a .of_match table but according the DT bindings, the
>>> compatible string "mediatek,mt6323-regulator" should be used so there
>>> should be a OF match table or the vendor prefix of the compatible
>>> string won't be used for matching (i.e: fallbacks to the driver .name
>>> for match).
> 
>> the driver is probed via drivers/mfd/mt6397-core.c and does not require
>> the OF match table. It loads fine just like the mt6397 driver.
> 
> That's fine but you shouldn't have the compatible string in your binding
> document since it's not actually used or needed.
> 
Hi,

correct me if i am wrong but if we remove the compatible string from the
binding document, then we will also have to remove it from the dts and
then the kernel won't be able to match the node to the driver and thus
the regulator phandle derefs will fail.

	John

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 13:35, Mark Brown wrote:
> On Mon, Jan 25, 2016 at 01:19:46PM +0100, John Crispin wrote:
>> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
> 
>>> You don't have a .of_match table but according the DT bindings, the
>>> compatible string "mediatek,mt6323-regulator" should be used so there
>>> should be a OF match table or the vendor prefix of the compatible
>>> string won't be used for matching (i.e: fallbacks to the driver .name
>>> for match).
> 
>> the driver is probed via drivers/mfd/mt6397-core.c and does not require
>> the OF match table. It loads fine just like the mt6397 driver.
> 
> That's fine but you shouldn't have the compatible string in your binding
> document since it's not actually used or needed.
> 
Hi,

correct me if i am wrong but if we remove the compatible string from the
binding document, then we will also have to remove it from the dts and
then the kernel won't be able to match the node to the driver and thus
the regulator phandle derefs will fail.

	John

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello John,

On Mon, Jan 25, 2016 at 10:13 AM, John Crispin <blogic@openwrt.org> wrote:
>
>
> On 25/01/2016 13:35, Mark Brown wrote:
>> On Mon, Jan 25, 2016 at 01:19:46PM +0100, John Crispin wrote:
>>> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
>>
>>>> You don't have a .of_match table but according the DT bindings, the
>>>> compatible string "mediatek,mt6323-regulator" should be used so there
>>>> should be a OF match table or the vendor prefix of the compatible
>>>> string won't be used for matching (i.e: fallbacks to the driver .name
>>>> for match).
>>
>>> the driver is probed via drivers/mfd/mt6397-core.c and does not require
>>> the OF match table. It loads fine just like the mt6397 driver.
>>
>> That's fine but you shouldn't have the compatible string in your binding
>> document since it's not actually used or needed.
>>
> Hi,
>
> correct me if i am wrong but if we remove the compatible string from the
> binding document, then we will also have to remove it from the dts and
> then the kernel won't be able to match the node to the driver and thus
> the regulator phandle derefs will fail.
>

The kernel doesn't need to match the compatible since the MFD driver
register the device explicitly with mfd_add_devices().

In fact, the kernel is currently not matching the compatible, it is
only matching because you provided a .of_compatible is provided in the
mfd_cell.

If you wan't subdevices for a MFD to be registered automatically by OF
and the compatible matched like other buses, then your MFD driver
needs to call of_platform_populate() instead mfd_add_devices().

>         John

Best regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello John,

On Mon, Jan 25, 2016 at 10:13 AM, John Crispin <blogic@openwrt.org> wrote:
>
>
> On 25/01/2016 13:35, Mark Brown wrote:
>> On Mon, Jan 25, 2016 at 01:19:46PM +0100, John Crispin wrote:
>>> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
>>
>>>> You don't have a .of_match table but according the DT bindings, the
>>>> compatible string "mediatek,mt6323-regulator" should be used so there
>>>> should be a OF match table or the vendor prefix of the compatible
>>>> string won't be used for matching (i.e: fallbacks to the driver .name
>>>> for match).
>>
>>> the driver is probed via drivers/mfd/mt6397-core.c and does not require
>>> the OF match table. It loads fine just like the mt6397 driver.
>>
>> That's fine but you shouldn't have the compatible string in your binding
>> document since it's not actually used or needed.
>>
> Hi,
>
> correct me if i am wrong but if we remove the compatible string from the
> binding document, then we will also have to remove it from the dts and
> then the kernel won't be able to match the node to the driver and thus
> the regulator phandle derefs will fail.
>

The kernel doesn't need to match the compatible since the MFD driver
register the device explicitly with mfd_add_devices().

In fact, the kernel is currently not matching the compatible, it is
only matching because you provided a .of_compatible is provided in the
mfd_cell.

If you wan't subdevices for a MFD to be registered automatically by OF
and the compatible matched like other buses, then your MFD driver
needs to call of_platform_populate() instead mfd_add_devices().

>         John

Best regards,
Javier

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello John,

On Mon, Jan 25, 2016 at 10:13 AM, John Crispin <blogic@openwrt.org> wrote:
>
>
> On 25/01/2016 13:35, Mark Brown wrote:
>> On Mon, Jan 25, 2016 at 01:19:46PM +0100, John Crispin wrote:
>>> On 25/01/2016 13:11, Javier Martinez Canillas wrote:
>>
>>>> You don't have a .of_match table but according the DT bindings, the
>>>> compatible string "mediatek,mt6323-regulator" should be used so there
>>>> should be a OF match table or the vendor prefix of the compatible
>>>> string won't be used for matching (i.e: fallbacks to the driver .name
>>>> for match).
>>
>>> the driver is probed via drivers/mfd/mt6397-core.c and does not require
>>> the OF match table. It loads fine just like the mt6397 driver.
>>
>> That's fine but you shouldn't have the compatible string in your binding
>> document since it's not actually used or needed.
>>
> Hi,
>
> correct me if i am wrong but if we remove the compatible string from the
> binding document, then we will also have to remove it from the dts and
> then the kernel won't be able to match the node to the driver and thus
> the regulator phandle derefs will fail.
>

The kernel doesn't need to match the compatible since the MFD driver
register the device explicitly with mfd_add_devices().

In fact, the kernel is currently not matching the compatible, it is
only matching because you provided a .of_compatible is provided in the
mfd_cell.

If you wan't subdevices for a MFD to be registered automatically by OF
and the compatible matched like other buses, then your MFD driver
needs to call of_platform_populate() instead mfd_add_devices().

>         John

Best regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:

[snip]

>
> In fact, the kernel is currently not matching the compatible, it is
> only matching because you provided a .of_compatible is provided in the
> mfd_cell.
>

Sorry my English was a bit off in this paragraph...

I tried to say that OF does not traverse MFD sub-devices and lookups a
device driver that matches the compatible automatically since a MFD
device is not a bus. Currently it is only trying to match a compatible
string because the mfd_cell has a .of_compatible set so an of_node is
assigned on mfd_add_device().

But it is failing to match because no OF device table is provided and
the platform bus match callback is falling back to the driver .name to
match so the compatible is not really used as Mark said.

Best regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:

[snip]

>
> In fact, the kernel is currently not matching the compatible, it is
> only matching because you provided a .of_compatible is provided in the
> mfd_cell.
>

Sorry my English was a bit off in this paragraph...

I tried to say that OF does not traverse MFD sub-devices and lookups a
device driver that matches the compatible automatically since a MFD
device is not a bus. Currently it is only trying to match a compatible
string because the mfd_cell has a .of_compatible set so an of_node is
assigned on mfd_add_device().

But it is failing to match because no OF device table is provided and
the platform bus match callback is falling back to the driver .name to
match so the compatible is not really used as Mark said.

Best regards,
Javier

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:

[snip]

>
> In fact, the kernel is currently not matching the compatible, it is
> only matching because you provided a .of_compatible is provided in the
> mfd_cell.
>

Sorry my English was a bit off in this paragraph...

I tried to say that OF does not traverse MFD sub-devices and lookups a
device driver that matches the compatible automatically since a MFD
device is not a bus. Currently it is only trying to match a compatible
string because the mfd_cell has a .of_compatible set so an of_node is
assigned on mfd_add_device().

But it is failing to match because no OF device table is provided and
the platform bus match callback is falling back to the driver .name to
match so the compatible is not really used as Mark said.

Best regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 14:25, Javier Martinez Canillas wrote:
> On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
> 
> [snip]
> 
>>
>> In fact, the kernel is currently not matching the compatible, it is
>> only matching because you provided a .of_compatible is provided in the
>> mfd_cell.
>>
> 
> Sorry my English was a bit off in this paragraph...
> 
> I tried to say that OF does not traverse MFD sub-devices and lookups a
> device driver that matches the compatible automatically since a MFD
> device is not a bus. Currently it is only trying to match a compatible
> string because the mfd_cell has a .of_compatible set so an of_node is
> assigned on mfd_add_device().
> 
> But it is failing to match because no OF device table is provided and
> the platform bus match callback is falling back to the driver .name to
> match so the compatible is not really used as Mark said.
> 
> Best regards,
> Javier
> 

Hi,

just so i am sure to have understood properly. i just need to drop the
compatible string from [1/2] and resend. if this is the case i will fix
the mt6397 binding doc while at it.

	John

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 14:25, Javier Martinez Canillas wrote:
> On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
> 
> [snip]
> 
>>
>> In fact, the kernel is currently not matching the compatible, it is
>> only matching because you provided a .of_compatible is provided in the
>> mfd_cell.
>>
> 
> Sorry my English was a bit off in this paragraph...
> 
> I tried to say that OF does not traverse MFD sub-devices and lookups a
> device driver that matches the compatible automatically since a MFD
> device is not a bus. Currently it is only trying to match a compatible
> string because the mfd_cell has a .of_compatible set so an of_node is
> assigned on mfd_add_device().
> 
> But it is failing to match because no OF device table is provided and
> the platform bus match callback is falling back to the driver .name to
> match so the compatible is not really used as Mark said.
> 
> Best regards,
> Javier
> 

Hi,

just so i am sure to have understood properly. i just need to drop the
compatible string from [1/2] and resend. if this is the case i will fix
the mt6397 binding doc while at it.

	John

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 14:25, Javier Martinez Canillas wrote:
> On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
> 
> [snip]
> 
>>
>> In fact, the kernel is currently not matching the compatible, it is
>> only matching because you provided a .of_compatible is provided in the
>> mfd_cell.
>>
> 
> Sorry my English was a bit off in this paragraph...
> 
> I tried to say that OF does not traverse MFD sub-devices and lookups a
> device driver that matches the compatible automatically since a MFD
> device is not a bus. Currently it is only trying to match a compatible
> string because the mfd_cell has a .of_compatible set so an of_node is
> assigned on mfd_add_device().
> 
> But it is failing to match because no OF device table is provided and
> the platform bus match callback is falling back to the driver .name to
> match so the compatible is not really used as Mark said.
> 
> Best regards,
> Javier
> 

Hi,

just so i am sure to have understood properly. i just need to drop the
compatible string from [1/2] and resend. if this is the case i will fix
the mt6397 binding doc while at it.

	John

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello John,

On Mon, Jan 25, 2016 at 10:46 AM, John Crispin <blogic@openwrt.org> wrote:
>
>
> On 25/01/2016 14:25, Javier Martinez Canillas wrote:
>> On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
>> <javier@dowhile0.org> wrote:
>>
>> [snip]
>>
>>>
>>> In fact, the kernel is currently not matching the compatible, it is
>>> only matching because you provided a .of_compatible is provided in the
>>> mfd_cell.
>>>
>>
>> Sorry my English was a bit off in this paragraph...
>>
>> I tried to say that OF does not traverse MFD sub-devices and lookups a
>> device driver that matches the compatible automatically since a MFD
>> device is not a bus. Currently it is only trying to match a compatible
>> string because the mfd_cell has a .of_compatible set so an of_node is
>> assigned on mfd_add_device().
>>
>> But it is failing to match because no OF device table is provided and
>> the platform bus match callback is falling back to the driver .name to
>> match so the compatible is not really used as Mark said.
>>
>> Best regards,
>> Javier
>>
>
> Hi,
>
> just so i am sure to have understood properly. i just need to drop the
> compatible string from [1/2] and resend. if this is the case i will fix
> the mt6397 binding doc while at it.
>

And you will also need to remove the .of_compatible =
"mediatek,mt6323-regulator" from patch "[PATCH V2 4/4] mfd: mediatek:
add MT6323 support to MT6397 driver" since otherwise an
MODALIAS=of:foo will be reported instead of an MODALIAS=platform:foo

But if I were you, I would keep the MFD driver and DT binding as they
are and provide a .id_table and .of_match_table to the mt6323
regulator platform driver.

I'll write patches for the mt6397 regulator driver adding those tables
since it has the same issue and you can see what I mean.

>         John

Best regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello John,

On Mon, Jan 25, 2016 at 10:46 AM, John Crispin <blogic@openwrt.org> wrote:
>
>
> On 25/01/2016 14:25, Javier Martinez Canillas wrote:
>> On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
>> <javier@dowhile0.org> wrote:
>>
>> [snip]
>>
>>>
>>> In fact, the kernel is currently not matching the compatible, it is
>>> only matching because you provided a .of_compatible is provided in the
>>> mfd_cell.
>>>
>>
>> Sorry my English was a bit off in this paragraph...
>>
>> I tried to say that OF does not traverse MFD sub-devices and lookups a
>> device driver that matches the compatible automatically since a MFD
>> device is not a bus. Currently it is only trying to match a compatible
>> string because the mfd_cell has a .of_compatible set so an of_node is
>> assigned on mfd_add_device().
>>
>> But it is failing to match because no OF device table is provided and
>> the platform bus match callback is falling back to the driver .name to
>> match so the compatible is not really used as Mark said.
>>
>> Best regards,
>> Javier
>>
>
> Hi,
>
> just so i am sure to have understood properly. i just need to drop the
> compatible string from [1/2] and resend. if this is the case i will fix
> the mt6397 binding doc while at it.
>

And you will also need to remove the .of_compatible =
"mediatek,mt6323-regulator" from patch "[PATCH V2 4/4] mfd: mediatek:
add MT6323 support to MT6397 driver" since otherwise an
MODALIAS=of:foo will be reported instead of an MODALIAS=platform:foo

But if I were you, I would keep the MFD driver and DT binding as they
are and provide a .id_table and .of_match_table to the mt6323
regulator platform driver.

I'll write patches for the mt6397 regulator driver adding those tables
since it has the same issue and you can see what I mean.

>         John

Best regards,
Javier

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[Javier Martinez Canillas]

Hello John,

On Mon, Jan 25, 2016 at 10:46 AM, John Crispin <blogic@openwrt.org> wrote:
>
>
> On 25/01/2016 14:25, Javier Martinez Canillas wrote:
>> On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
>> <javier@dowhile0.org> wrote:
>>
>> [snip]
>>
>>>
>>> In fact, the kernel is currently not matching the compatible, it is
>>> only matching because you provided a .of_compatible is provided in the
>>> mfd_cell.
>>>
>>
>> Sorry my English was a bit off in this paragraph...
>>
>> I tried to say that OF does not traverse MFD sub-devices and lookups a
>> device driver that matches the compatible automatically since a MFD
>> device is not a bus. Currently it is only trying to match a compatible
>> string because the mfd_cell has a .of_compatible set so an of_node is
>> assigned on mfd_add_device().
>>
>> But it is failing to match because no OF device table is provided and
>> the platform bus match callback is falling back to the driver .name to
>> match so the compatible is not really used as Mark said.
>>
>> Best regards,
>> Javier
>>
>
> Hi,
>
> just so i am sure to have understood properly. i just need to drop the
> compatible string from [1/2] and resend. if this is the case i will fix
> the mt6397 binding doc while at it.
>

And you will also need to remove the .of_compatible =
"mediatek,mt6323-regulator" from patch "[PATCH V2 4/4] mfd: mediatek:
add MT6323 support to MT6397 driver" since otherwise an
MODALIAS=of:foo will be reported instead of an MODALIAS=platform:foo

But if I were you, I would keep the MFD driver and DT binding as they
are and provide a .id_table and .of_match_table to the mt6323
regulator platform driver.

I'll write patches for the mt6397 regulator driver adding those tables
since it has the same issue and you can see what I mean.

>         John

Best regards,
Javier

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 15:01, Javier Martinez Canillas wrote:
> Hello John,
> 
> On Mon, Jan 25, 2016 at 10:46 AM, John Crispin <blogic@openwrt.org> wrote:
>>
>>
>> On 25/01/2016 14:25, Javier Martinez Canillas wrote:
>>> On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
>>> <javier@dowhile0.org> wrote:
>>>
>>> [snip]
>>>
>>>>
>>>> In fact, the kernel is currently not matching the compatible, it is
>>>> only matching because you provided a .of_compatible is provided in the
>>>> mfd_cell.
>>>>
>>>
>>> Sorry my English was a bit off in this paragraph...
>>>
>>> I tried to say that OF does not traverse MFD sub-devices and lookups a
>>> device driver that matches the compatible automatically since a MFD
>>> device is not a bus. Currently it is only trying to match a compatible
>>> string because the mfd_cell has a .of_compatible set so an of_node is
>>> assigned on mfd_add_device().
>>>
>>> But it is failing to match because no OF device table is provided and
>>> the platform bus match callback is falling back to the driver .name to
>>> match so the compatible is not really used as Mark said.
>>>
>>> Best regards,
>>> Javier
>>>
>>
>> Hi,
>>
>> just so i am sure to have understood properly. i just need to drop the
>> compatible string from [1/2] and resend. if this is the case i will fix
>> the mt6397 binding doc while at it.
>>
> 
> And you will also need to remove the .of_compatible =
> "mediatek,mt6323-regulator" from patch "[PATCH V2 4/4] mfd: mediatek:
> add MT6323 support to MT6397 driver" since otherwise an
> MODALIAS=of:foo will be reported instead of an MODALIAS=platform:foo
> 
> But if I were you, I would keep the MFD driver and DT binding as they
> are and provide a .id_table and .of_match_table to the mt6323
> regulator platform driver.
> 
> I'll write patches for the mt6397 regulator driver adding those tables
> since it has the same issue and you can see what I mean.
> 
>>         John
> 
> Best regards,
> Javier
> 

Hi Javier,

fine i'll do that then. thanks for the elaborate explanation.

	John

Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 15:01, Javier Martinez Canillas wrote:
> Hello John,
> 
> On Mon, Jan 25, 2016 at 10:46 AM, John Crispin <blogic@openwrt.org> wrote:
>>
>>
>> On 25/01/2016 14:25, Javier Martinez Canillas wrote:
>>> On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
>>> <javier@dowhile0.org> wrote:
>>>
>>> [snip]
>>>
>>>>
>>>> In fact, the kernel is currently not matching the compatible, it is
>>>> only matching because you provided a .of_compatible is provided in the
>>>> mfd_cell.
>>>>
>>>
>>> Sorry my English was a bit off in this paragraph...
>>>
>>> I tried to say that OF does not traverse MFD sub-devices and lookups a
>>> device driver that matches the compatible automatically since a MFD
>>> device is not a bus. Currently it is only trying to match a compatible
>>> string because the mfd_cell has a .of_compatible set so an of_node is
>>> assigned on mfd_add_device().
>>>
>>> But it is failing to match because no OF device table is provided and
>>> the platform bus match callback is falling back to the driver .name to
>>> match so the compatible is not really used as Mark said.
>>>
>>> Best regards,
>>> Javier
>>>
>>
>> Hi,
>>
>> just so i am sure to have understood properly. i just need to drop the
>> compatible string from [1/2] and resend. if this is the case i will fix
>> the mt6397 binding doc while at it.
>>
> 
> And you will also need to remove the .of_compatible =
> "mediatek,mt6323-regulator" from patch "[PATCH V2 4/4] mfd: mediatek:
> add MT6323 support to MT6397 driver" since otherwise an
> MODALIAS=of:foo will be reported instead of an MODALIAS=platform:foo
> 
> But if I were you, I would keep the MFD driver and DT binding as they
> are and provide a .id_table and .of_match_table to the mt6323
> regulator platform driver.
> 
> I'll write patches for the mt6397 regulator driver adding those tables
> since it has the same issue and you can see what I mean.
> 
>>         John
> 
> Best regards,
> Javier
> 

Hi Javier,

fine i'll do that then. thanks for the elaborate explanation.

	John

[PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator

[John Crispin]

On 25/01/2016 15:01, Javier Martinez Canillas wrote:
> Hello John,
> 
> On Mon, Jan 25, 2016 at 10:46 AM, John Crispin <blogic@openwrt.org> wrote:
>>
>>
>> On 25/01/2016 14:25, Javier Martinez Canillas wrote:
>>> On Mon, Jan 25, 2016 at 10:19 AM, Javier Martinez Canillas
>>> <javier@dowhile0.org> wrote:
>>>
>>> [snip]
>>>
>>>>
>>>> In fact, the kernel is currently not matching the compatible, it is
>>>> only matching because you provided a .of_compatible is provided in the
>>>> mfd_cell.
>>>>
>>>
>>> Sorry my English was a bit off in this paragraph...
>>>
>>> I tried to say that OF does not traverse MFD sub-devices and lookups a
>>> device driver that matches the compatible automatically since a MFD
>>> device is not a bus. Currently it is only trying to match a compatible
>>> string because the mfd_cell has a .of_compatible set so an of_node is
>>> assigned on mfd_add_device().
>>>
>>> But it is failing to match because no OF device table is provided and
>>> the platform bus match callback is falling back to the driver .name to
>>> match so the compatible is not really used as Mark said.
>>>
>>> Best regards,
>>> Javier
>>>
>>
>> Hi,
>>
>> just so i am sure to have understood properly. i just need to drop the
>> compatible string from [1/2] and resend. if this is the case i will fix
>> the mt6397 binding doc while at it.
>>
> 
> And you will also need to remove the .of_compatible =
> "mediatek,mt6323-regulator" from patch "[PATCH V2 4/4] mfd: mediatek:
> add MT6323 support to MT6397 driver" since otherwise an
> MODALIAS=of:foo will be reported instead of an MODALIAS=platform:foo
> 
> But if I were you, I would keep the MFD driver and DT binding as they
> are and provide a .id_table and .of_match_table to the mt6323
> regulator platform driver.
> 
> I'll write patches for the mt6397 regulator driver adding those tables
> since it has the same issue and you can see what I mean.
> 
>>         John
> 
> Best regards,
> Javier
> 

Hi Javier,

fine i'll do that then. thanks for the elaborate explanation.

	John