| Message ID | 1387371179-16725-2-git-send-email-ldewangan@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/18/2013 05:52 AM, Laxman Dewangan wrote: > Add ams AS3722 entry for gpio/pincontrol and regulators > to venice2 DT. This patch still causes: > [ 0.726545] as3722-pinctrl as3722-pinctrl: pin gpio0 already requested by as3722-pinctrl; cannot claim for as3722-regulator > [ 0.737681] as3722-pinctrl as3722-pinctrl: pin-0 (as3722-regulator) status -22 > [ 0.744895] as3722-pinctrl as3722-pinctrl: could not request pin 0 (gpio0) from group gpio0 on device as3722-pinctrl > [ 0.755500] as3722-regulator as3722-regulator: Error applying setting, reverse things back
On Thursday 19 December 2013 02:25 AM, Stephen Warren wrote: > On 12/18/2013 05:52 AM, Laxman Dewangan wrote: >> Add ams AS3722 entry for gpio/pincontrol and regulators >> to venice2 DT. > This patch still causes: > >> [ 0.726545] as3722-pinctrl as3722-pinctrl: pin gpio0 already requested by as3722-pinctrl; cannot claim for as3722-regulator >> [ 0.737681] as3722-pinctrl as3722-pinctrl: pin-0 (as3722-regulator) status -22 >> [ 0.744895] as3722-pinctrl as3722-pinctrl: could not request pin 0 (gpio0) from group gpio0 on device as3722-pinctrl >> [ 0.755500] as3722-regulator as3722-regulator: Error applying setting, reverse things back This error is nothing related to the ams dt or driver. This came from the framework from the driver/base for adding pinmux call before calling any diver's probe. Here is my finding: Frameworks calls the pinctrl_bind_pins() before calls the driver's probe (drivers/base/dd.c) and pinctrl_bind_pins() calls the pinmux mapping and try to set the default (drvers/base/pinctrl.c). AMS AS3722 DT is flat type on which all sub devices of AS3722 have the property on parent node only, there is no subnode for each sub driver like palmas. In this case all sub drivers of_node is initialized as parent->of_node in mfd-core.c (mfd_add_devices). mfd-core.c: mfd_add_device() if (!pdev->dev.of_node) pdev->dev.of_node = parent->of_node; So all sub devices of AS3722 has the parent node. This node has the default pinmux configuration and so it tries to set the default pinmux before sud-driver's probe get called. The first one (for pinmux driver) it is success but for regulator and rtc, it is failed as pins are owned by the pinmux drivers. This does not have any side impact as currently, pinctrl_bind_pins() only return the success or ERPOBE_DEFER I was testing the patch with linux-next-common of your branch and chrome-os/kernel-next with linux-next's pinctrl/regulator/mfd and not able to repro this because driver/base changes was not there.
On 12/19/2013 12:28 AM, Laxman Dewangan wrote: > On Thursday 19 December 2013 02:25 AM, Stephen Warren wrote: >> On 12/18/2013 05:52 AM, Laxman Dewangan wrote: >>> Add ams AS3722 entry for gpio/pincontrol and regulators >>> to venice2 DT. >> This patch still causes: >> >>> [ 0.726545] as3722-pinctrl as3722-pinctrl: pin gpio0 already >>> requested by as3722-pinctrl; cannot claim for as3722-regulator >>> [ 0.737681] as3722-pinctrl as3722-pinctrl: pin-0 >>> (as3722-regulator) status -22 >>> [ 0.744895] as3722-pinctrl as3722-pinctrl: could not request pin 0 >>> (gpio0) from group gpio0 on device as3722-pinctrl >>> [ 0.755500] as3722-regulator as3722-regulator: Error applying >>> setting, reverse things back > > This error is nothing related to the ams dt or driver. This came from > the framework from the driver/base for adding pinmux call before calling > any diver's probe. > Here is my finding: > Frameworks calls the pinctrl_bind_pins() before calls the driver's probe > (drivers/base/dd.c) and pinctrl_bind_pins() calls the pinmux mapping and > try to set the default (drvers/base/pinctrl.c). > > AMS AS3722 DT is flat type on which all sub devices of AS3722 have the > property on parent node only, there is no subnode for each sub driver > like palmas. In this case all sub drivers of_node is initialized as > parent->of_node in mfd-core.c (mfd_add_devices). > > mfd-core.c: mfd_add_device() > if (!pdev->dev.of_node) > pdev->dev.of_node = parent->of_node; > > So all sub devices of AS3722 has the parent node. > This node has the default pinmux configuration and so it tries to set > the default pinmux before sud-driver's probe get called. > The first one (for pinmux driver) it is success but for regulator and > rtc, it is failed as pins are owned by the pinmux drivers. Ah. I guess we should drop/revert 1c79a8b9f4a7 "mfd: Always assign of_node in mfd_add_device()" then. I guess I'll make the RTC core look at dev->of_node, or dev->parent->of_node to search for aliases. That should avoid the issue.
On 12/19/2013 12:28 AM, Laxman Dewangan wrote: > On Thursday 19 December 2013 02:25 AM, Stephen Warren wrote: >> On 12/18/2013 05:52 AM, Laxman Dewangan wrote: >>> Add ams AS3722 entry for gpio/pincontrol and regulators >>> to venice2 DT. >> This patch still causes: >> >>> [ 0.726545] as3722-pinctrl as3722-pinctrl: pin gpio0 already >>> requested by as3722-pinctrl; cannot claim for as3722-regulator >>> [ 0.737681] as3722-pinctrl as3722-pinctrl: pin-0 >>> (as3722-regulator) status -22 >>> [ 0.744895] as3722-pinctrl as3722-pinctrl: could not request pin 0 >>> (gpio0) from group gpio0 on device as3722-pinctrl >>> [ 0.755500] as3722-regulator as3722-regulator: Error applying >>> setting, reverse things back > > This error is nothing related to the ams dt or driver. This came from > the framework from the driver/base for adding pinmux call before calling > any diver's probe. OK, after reverting the problematic MFD commit (which I assume will happen upstream too), I see no errors caused by this patch. As such, I've applied the series to Tegra's for-3.14/dt branch. Note that I couldn't verify if the pinctrl setup in patch 1/2 matched the ChromeOS kernel, since the node order is different there, and the property values don't use named constants there. I'll have to trust you. I'll be annoyed if it turns out we need to churn the pinctrl configuration in the future...
On Thursday 19 December 2013 11:53 PM, Stephen Warren wrote: > On 12/19/2013 12:28 AM, Laxman Dewangan wrote: >> On Thursday 19 December 2013 02:25 AM, Stephen Warren wrote: >>> On 12/18/2013 05:52 AM, Laxman Dewangan wrote: >>>> Add ams AS3722 entry for gpio/pincontrol and regulators >>>> to venice2 DT. >>> This patch still causes: >>> >>>> [ 0.726545] as3722-pinctrl as3722-pinctrl: pin gpio0 already >>>> requested by as3722-pinctrl; cannot claim for as3722-regulator >>>> [ 0.737681] as3722-pinctrl as3722-pinctrl: pin-0 >>>> (as3722-regulator) status -22 >>>> [ 0.744895] as3722-pinctrl as3722-pinctrl: could not request pin 0 >>>> (gpio0) from group gpio0 on device as3722-pinctrl >>>> [ 0.755500] as3722-regulator as3722-regulator: Error applying >>>> setting, reverse things back >> This error is nothing related to the ams dt or driver. This came from >> the framework from the driver/base for adding pinmux call before calling >> any diver's probe. > OK, after reverting the problematic MFD commit (which I assume will > happen upstream too), I see no errors caused by this patch. > > As such, I've applied the series to Tegra's for-3.14/dt branch. Thanks for taking care. > > Note that I couldn't verify if the pinctrl setup in patch 1/2 matched > the ChromeOS kernel, since the node order is different there, and the > property values don't use named constants there. I'll have to trust you. > I'll be annoyed if it turns out we need to churn the pinctrl > configuration in the future... I referred the configuration from chrome branch. I assume that good amount of testing has already been done on this configuration. I manually check all configuration and run chrome on this change which works properly. Did not see any abnormal behavior. So if something missed then it is just human error which did not catch on 2 level of self review. Again any change on this configuration can be expected based on new testing or bringung new modules.
diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts index 6bc4e07..a9f0df9 100644 --- a/arch/arm/boot/dts/tegra124-venice2.dts +++ b/arch/arm/boot/dts/tegra124-venice2.dts @@ -603,7 +603,201 @@ i2c@7000d000 { status = "okay"; - clock-frequency = <100000>; + clock-frequency = <400000>; + + as3722: as3722@40 { + compatible = "ams,as3722"; + reg = <0x40>; + interrupts = <0 86 IRQ_TYPE_LEVEL_HIGH>; + + #interrupt-cells = <2>; + interrupt-controller; + + gpio-controller; + #gpio-cells = <2>; + + pinctrl-names = "default"; + pinctrl-0 = <&as3722_default>; + + as3722_default: pinmux { + gpio0 { + pins = "gpio0"; + function = "gpio"; + bias-pull-down; + }; + + gpio1_2_4_7 { + pins = "gpio1", "gpio2", "gpio4", "gpio7"; + function = "gpio"; + bias-pull-up; + }; + + gpio3_6 { + pins = "gpio3", "gpio6"; + bias-high-impedance; + }; + + gpio5 { + pins = "gpio5"; + function = "clk32k-out"; + }; + }; + + regulators { + vsup-sd2-supply = <&vdd_ac_bat_reg>; + vsup-sd3-supply = <&vdd_ac_bat_reg>; + vsup-sd4-supply = <&vdd_ac_bat_reg>; + vsup-sd5-supply = <&vdd_ac_bat_reg>; + vin-ldo0-supply = <&as3722_sd2>; + vin-ldo1-6-supply = <&vdd_ac_bat_reg>; + vin-ldo2-5-7-supply = <&as3722_sd5>; + vin-ldo3-4-supply = <&vdd_ac_bat_reg>; + vin-ldo9-10-supply = <&vdd_ac_bat_reg>; + vin-ldo11-supply = <&vdd_ac_bat_reg>; + + sd0 { + regulator-name = "vdd-cpu"; + regulator-min-microvolt = <700000>; + regulator-max-microvolt = <1400000>; + regulator-min-microamp = <3500000>; + regulator-max-microamp = <3500000>; + regulator-always-on; + regulator-boot-on; + ams,external-control = <2>; + }; + + sd1 { + regulator-name = "vdd-core"; + regulator-min-microvolt = <700000>; + regulator-max-microvolt = <1350000>; + regulator-min-microamp = <2500000>; + regulator-max-microamp = <2500000>; + regulator-always-on; + regulator-boot-on; + ams,external-control = <1>; + }; + + as3722_sd2: sd2 { + regulator-name = "vddio-ddr"; + regulator-min-microvolt = <1350000>; + regulator-max-microvolt = <1350000>; + regulator-always-on; + regulator-boot-on; + }; + + sd3 { + regulator-name = "vddio-ddr-2phase"; + regulator-min-microvolt = <1350000>; + regulator-max-microvolt = <1350000>; + regulator-always-on; + regulator-boot-on; + }; + + sd4 { + regulator-name = "avdd-pex-sata"; + regulator-min-microvolt = <1050000>; + regulator-max-microvolt = <1050000>; + regulator-boot-on; + regulator-always-on; + }; + + as3722_sd5: sd5 { + regulator-name = "vddio-sys"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-boot-on; + regulator-always-on; + }; + + sd6 { + regulator-name = "vdd-gpu"; + regulator-min-microvolt = <650000>; + regulator-max-microvolt = <1200000>; + regulator-min-microamp = <3500000>; + regulator-max-microamp = <3500000>; + regulator-boot-on; + regulator-always-on; + }; + + ldo0 { + regulator-name = "avdd_pll"; + regulator-min-microvolt = <1050000>; + regulator-max-microvolt = <1050000>; + regulator-boot-on; + regulator-always-on; + ams,external-control = <1>; + }; + + ldo1 { + regulator-name = "run-cam-1.8"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + + ldo2 { + regulator-name = "gen-avdd,vddio-hsic"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-boot-on; + regulator-always-on; + }; + + ldo3 { + regulator-name = "vdd-rtc"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-boot-on; + regulator-always-on; + ams,enable-tracking; + }; + + ldo4 { + regulator-name = "vdd-cam"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-boot-on; + regulator-always-on; + }; + + ldo5 { + regulator-name = "vdd-cam-front"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + }; + + ldo6 { + regulator-name = "vddio-sdmmc3"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + regulator-boot-on; + regulator-always-on; + }; + + ldo7 { + regulator-name = "vdd-cam-rear"; + regulator-min-microvolt = <1050000>; + regulator-max-microvolt = <1050000>; + }; + + ldo9 { + regulator-name = "vdd-touch"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + }; + + ldo10 { + regulator-name = "vdd-cam-af"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + }; + + ldo11 { + regulator-name = "vpp-fuse"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + }; + }; + }; }; pmc@7000e400 { @@ -648,6 +842,107 @@ }; }; + regulators { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <0>; + + vdd_ac_bat_reg: regulator@0 { + compatible = "regulator-fixed"; + reg = <0>; + regulator-name = "vdd_ac_bat"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + regulator-always-on; + }; + + vdd_3v3_reg: regulator@1 { + compatible = "regulator-fixed"; + reg = <1>; + regulator-name = "vdd_3v3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + regulator-boot-on; + enable-active-high; + gpio = <&as3722 1 GPIO_ACTIVE_HIGH>; + }; + + vdd_3v3_modem_reg: regulator@2 { + compatible = "regulator-fixed"; + reg = <2>; + regulator-name = "vdd-modem-3v3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + enable-active-high; + gpio = <&as3722 2 GPIO_ACTIVE_HIGH>; + }; + + vdd_hdmi_5v0_reg: regulator@3 { + compatible = "regulator-fixed"; + reg = <3>; + regulator-name = "vdd-hdmi-5v0"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + enable-active-high; + gpio = <&gpio TEGRA_GPIO(K, 6) GPIO_ACTIVE_HIGH>; + }; + + vdd_bl_reg: regulator@4 { + compatible = "regulator-fixed"; + reg = <4>; + regulator-name = "vdd-bl"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + gpio = <&gpio TEGRA_GPIO(P, 2) GPIO_ACTIVE_LOW>; + }; + + vdd_ts_sw_5v0: regulator@5 { + compatible = "regulator-fixed"; + reg = <5>; + regulator-name = "vdd_ts_sw"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + enable-active-high; + regulator-boot-on; + gpio = <&gpio TEGRA_GPIO(K, 1) GPIO_ACTIVE_LOW>; + }; + + usb1_vbus_reg: regulator@6 { + compatible = "regulator-fixed"; + reg = <6>; + regulator-name = "usb1_vbus"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + regulator-boot-on; + enable-active-high; + gpio = <&gpio TEGRA_GPIO(N, 4) GPIO_ACTIVE_HIGH>; + gpio-open-drain; + }; + + usb3_vbus_reg: regulator@7 { + compatible = "regulator-fixed"; + reg = <7>; + regulator-name = "usb3_vbus"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + regulator-boot-on; + enable-active-high; + gpio = <&gpio TEGRA_GPIO(N, 5) GPIO_ACTIVE_HIGH>; + gpio-open-drain; + }; + + panel_3v3_reg: regulator@8 { + compatible = "regulator-fixed"; + reg = <8>; + regulator-name = "panel_3v3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + enable-active-high; + gpio = <&as3722 4 GPIO_ACTIVE_HIGH>; + }; + }; + sound { compatible = "nvidia,tegra-audio-max98090-venice2", "nvidia,tegra-audio-max98090";
Add ams AS3722 entry for gpio/pincontrol and regulators to venice2 DT. Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> --- Changes fome V1: - Separte the pinmux change on different patch. - Compare the conifguration with chorme internal branch for Venice2. arch/arm/boot/dts/tegra124-venice2.dts | 297 +++++++++++++++++++++++++++++++- 1 file changed, 296 insertions(+), 1 deletion(-)