* [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator @ 2016-01-25 10:40 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 10:40 UTC (permalink / raw) To: Liam Girdwood, Mark Brown Cc: Chen Zhong, Matthias Brugger, Henry Chen, Steven Liu, linux-mediatek, linux-arm-kernel, linux-kernel, John Crispin, devicetree 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 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator @ 2016-01-25 10:40 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 10:40 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 10:40 ` John Crispin @ 2016-01-25 10:40 ` John Crispin -1 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 10:40 UTC (permalink / raw) To: Liam Girdwood, Mark Brown Cc: Chen Zhong, Matthias Brugger, Henry Chen, Steven Liu, linux-mediatek, linux-arm-kernel, linux-kernel, 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, ®val); + 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, + ®val) < 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, ®_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 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 10:40 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 10:40 UTC (permalink / raw) To: linux-arm-kernel 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, ®val); + 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, + ®val) < 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, ®_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 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 10:40 ` John Crispin (?) @ 2016-01-25 12:11 ` Javier Martinez Canillas -1 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 12:11 UTC (permalink / raw) To: John Crispin Cc: Liam Girdwood, Mark Brown, Steven Liu, Linux Kernel, Henry Chen, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 12:11 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 12:11 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 12:11 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 12:11 UTC (permalink / raw) To: John Crispin Cc: Liam Girdwood, Mark Brown, Steven Liu, Linux Kernel, Henry Chen, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 12:11 ` Javier Martinez Canillas (?) @ 2016-01-25 12:19 ` John Crispin -1 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 12:19 UTC (permalink / raw) To: Javier Martinez Canillas Cc: Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, Mark Brown, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 12:19 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 12:19 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 12:19 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 12:19 UTC (permalink / raw) To: Javier Martinez Canillas Cc: Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, Mark Brown, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 12:19 ` John Crispin (?) @ 2016-01-25 12:30 ` Javier Martinez Canillas -1 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 12:30 UTC (permalink / raw) To: John Crispin Cc: Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, Mark Brown, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 12:30 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 12:30 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 12:30 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 12:30 UTC (permalink / raw) To: John Crispin Cc: Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, Mark Brown, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <CABxcv=kC3tOrfJvCCaXcMSO4ufBXc_GoWeGBcX56k1RV-BX=ow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator [not found] ` <CABxcv=kC3tOrfJvCCaXcMSO4ufBXc_GoWeGBcX56k1RV-BX=ow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2016-01-25 12:33 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 12:33 UTC (permalink / raw) To: linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r 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 > ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 12:19 ` John Crispin (?) @ 2016-01-25 12:35 ` Mark Brown -1 siblings, 0 replies; 41+ messages in thread From: Mark Brown @ 2016-01-25 12:35 UTC (permalink / raw) To: John Crispin Cc: Javier Martinez Canillas, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 12:35 ` Mark Brown 0 siblings, 0 replies; 41+ messages in thread From: Mark Brown @ 2016-01-25 12:35 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 12:35 ` Mark Brown 0 siblings, 0 replies; 41+ messages in thread From: Mark Brown @ 2016-01-25 12:35 UTC (permalink / raw) To: John Crispin Cc: Javier Martinez Canillas, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 12:35 ` Mark Brown (?) @ 2016-01-25 13:13 ` John Crispin -1 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 13:13 UTC (permalink / raw) To: Mark Brown Cc: Javier Martinez Canillas, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 13:13 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 13:13 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 13:13 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 13:13 UTC (permalink / raw) To: Mark Brown Cc: Javier Martinez Canillas, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 13:13 ` John Crispin (?) @ 2016-01-25 13:19 ` Javier Martinez Canillas -1 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 13:19 UTC (permalink / raw) To: John Crispin Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 13:19 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 13:19 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 13:19 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 13:19 UTC (permalink / raw) To: John Crispin Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 13:19 ` Javier Martinez Canillas (?) @ 2016-01-25 13:25 ` Javier Martinez Canillas -1 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 13:25 UTC (permalink / raw) To: John Crispin Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 13:25 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 13:25 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 13:25 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 13:25 UTC (permalink / raw) To: John Crispin Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 13:25 ` Javier Martinez Canillas (?) @ 2016-01-25 13:46 ` John Crispin -1 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 13:46 UTC (permalink / raw) To: Javier Martinez Canillas Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 13:46 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 13:46 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 13:46 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 13:46 UTC (permalink / raw) To: Javier Martinez Canillas Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 13:46 ` John Crispin (?) @ 2016-01-25 14:01 ` Javier Martinez Canillas -1 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 14:01 UTC (permalink / raw) To: John Crispin Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 14:01 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 14:01 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 14:01 ` Javier Martinez Canillas 0 siblings, 0 replies; 41+ messages in thread From: Javier Martinez Canillas @ 2016-01-25 14:01 UTC (permalink / raw) To: John Crispin Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator 2016-01-25 14:01 ` Javier Martinez Canillas (?) @ 2016-01-25 14:02 ` John Crispin -1 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 14:02 UTC (permalink / raw) To: Javier Martinez Canillas Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 14:02 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 14:02 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 2/2] regulator: mt6323: Add support for MT6323 regulator @ 2016-01-25 14:02 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 14:02 UTC (permalink / raw) To: Javier Martinez Canillas Cc: Mark Brown, Steven Liu, Linux Kernel, Henry Chen, Liam Girdwood, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator @ 2016-01-25 11:37 ` Mark Brown 0 siblings, 0 replies; 41+ messages in thread From: Mark Brown @ 2016-01-25 11:37 UTC (permalink / raw) To: John Crispin Cc: Liam Girdwood, Chen Zhong, Matthias Brugger, Henry Chen, Steven Liu, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree [-- 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 --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator @ 2016-01-25 11:37 ` Mark Brown 0 siblings, 0 replies; 41+ messages in thread From: Mark Brown @ 2016-01-25 11:37 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator @ 2016-01-25 11:37 ` Mark Brown 0 siblings, 0 replies; 41+ messages in thread From: Mark Brown @ 2016-01-25 11:37 UTC (permalink / raw) To: John Crispin Cc: Liam Girdwood, Chen Zhong, Matthias Brugger, Henry Chen, Steven Liu, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA [-- 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 --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator @ 2016-01-25 12:05 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 12:05 UTC (permalink / raw) To: Mark Brown Cc: devicetree, Steven Liu, Liam Girdwood, Henry Chen, linux-kernel, linux-mediatek, Matthias Brugger, Chen Zhong, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator @ 2016-01-25 12:05 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 12:05 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator @ 2016-01-25 12:05 ` John Crispin 0 siblings, 0 replies; 41+ messages in thread From: John Crispin @ 2016-01-25 12:05 UTC (permalink / raw) To: Mark Brown Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Steven Liu, Liam Girdwood, Henry Chen, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger, Chen Zhong, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2016-01-25 14:03 UTC | newest] Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-01-25 10:40 [PATCH V2 1/2] dt-bindings: regulator: Add document for MT6323 regulator John Crispin 2016-01-25 10:40 ` John Crispin 2016-01-25 10:40 ` [PATCH V2 2/2] regulator: mt6323: Add support " John Crispin 2016-01-25 10:40 ` John Crispin 2016-01-25 12:11 ` Javier Martinez Canillas 2016-01-25 12:11 ` Javier Martinez Canillas 2016-01-25 12:11 ` Javier Martinez Canillas 2016-01-25 12:19 ` John Crispin 2016-01-25 12:19 ` John Crispin 2016-01-25 12:19 ` John Crispin 2016-01-25 12:30 ` Javier Martinez Canillas 2016-01-25 12:30 ` Javier Martinez Canillas 2016-01-25 12:30 ` Javier Martinez Canillas [not found] ` <CABxcv=kC3tOrfJvCCaXcMSO4ufBXc_GoWeGBcX56k1RV-BX=ow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-01-25 12:33 ` John Crispin 2016-01-25 12:35 ` Mark Brown 2016-01-25 12:35 ` Mark Brown 2016-01-25 12:35 ` Mark Brown 2016-01-25 13:13 ` John Crispin 2016-01-25 13:13 ` John Crispin 2016-01-25 13:13 ` John Crispin 2016-01-25 13:19 ` Javier Martinez Canillas 2016-01-25 13:19 ` Javier Martinez Canillas 2016-01-25 13:19 ` Javier Martinez Canillas 2016-01-25 13:25 ` Javier Martinez Canillas 2016-01-25 13:25 ` Javier Martinez Canillas 2016-01-25 13:25 ` Javier Martinez Canillas 2016-01-25 13:46 ` John Crispin 2016-01-25 13:46 ` John Crispin 2016-01-25 13:46 ` John Crispin 2016-01-25 14:01 ` Javier Martinez Canillas 2016-01-25 14:01 ` Javier Martinez Canillas 2016-01-25 14:01 ` Javier Martinez Canillas 2016-01-25 14:02 ` John Crispin 2016-01-25 14:02 ` John Crispin 2016-01-25 14:02 ` John Crispin 2016-01-25 11:37 ` [PATCH V2 1/2] dt-bindings: regulator: Add document " Mark Brown 2016-01-25 11:37 ` Mark Brown 2016-01-25 11:37 ` Mark Brown 2016-01-25 12:05 ` John Crispin 2016-01-25 12:05 ` John Crispin 2016-01-25 12:05 ` John Crispin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.