| Message ID | 1350626311-18131-1-git-send-email-markz@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/18/2012 11:58 PM, Mark Zhang wrote: > Define pinmux for DDC. The DDC pinmux in Ventana is 2 pins in I2C2. > +++ b/arch/arm/boot/dts/tegra20-ventana.dts > - ddc { > - nvidia,pins = "ddc", "owc", "spdi", "spdo", > - "uac"; > - nvidia,function = "rsvd2"; > - }; So that removes the entries for 5 pin groups, yet below, entries are only added for the ddc and pta pingroups, so the other 4 pin groups become unconfigured. > + > + state_i2cmux_ddc: pinmux_i2cmux_ddc { > + ddc { > + nvidia,pins = "ddc"; > + nvidia,function = "i2c2"; > + }; > + pta { > + nvidia,pins = "pta"; > + nvidia,function = "rsvd4"; > + }; Does this actually work? The pta pingroup is configured by the "hog" pinctrl state of the pinctrl node itself, so this state should fail to be applied since it attempts to touch the same pingroup. > + i2cmux { ... > + i2c@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + smart-battery@b { > + compatible = "ti,bq24617", "smart-battery-1.1"; > + reg = <0xb>; > + ti,i2c-retry-count = <2>; > + ti,poll-retry-count = <10>; > + }; That wasn't there before. Does that device actually exist on Ventana? Adding it should be a separate patch if so, since this one is just about introducing the I2C mux, not adding an SBS device. Was this all just cut/paste from Seaboard without validation?
On 10/19/2012 11:48 PM, Stephen Warren wrote: > On 10/18/2012 11:58 PM, Mark Zhang wrote: >> Define pinmux for DDC. The DDC pinmux in Ventana is 2 pins in I2C2. > >> +++ b/arch/arm/boot/dts/tegra20-ventana.dts > >> - ddc { >> - nvidia,pins = "ddc", "owc", "spdi", "spdo", >> - "uac"; >> - nvidia,function = "rsvd2"; >> - }; > > So that removes the entries for 5 pin groups, yet below, entries are > only added for the ddc and pta pingroups, so the other 4 pin groups > become unconfigured. > Right. So I think it should be changed to: owc { nvidia,pins = "owc", "spdi", "spdo", "uac"; nvidia,function = "rsvd2"; }; Is this right? >> + >> + state_i2cmux_ddc: pinmux_i2cmux_ddc { >> + ddc { >> + nvidia,pins = "ddc"; >> + nvidia,function = "i2c2"; >> + }; >> + pta { >> + nvidia,pins = "pta"; >> + nvidia,function = "rsvd4"; >> + }; > > Does this actually work? The pta pingroup is configured by the "hog" > pinctrl state of the pinctrl node itself, so this state should fail to > be applied since it attempts to touch the same pingroup. > I know little about kernel pinctrl subsystem. After reading some docs and codes, I think what you mean is, in Ventana's pinmux configuration, pta pingroup has been defined as "hdmi" function and this can't be changed(non-dynamic pinmuxing). So I want to know why we have defined pta pingroup as hdmi function? Can we remove this definition to make the i2cmux above working? >> + i2cmux { > ... >> + i2c@1 { >> + reg = <1>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + smart-battery@b { >> + compatible = "ti,bq24617", "smart-battery-1.1"; >> + reg = <0xb>; >> + ti,i2c-retry-count = <2>; >> + ti,poll-retry-count = <10>; >> + }; > > That wasn't there before. Does that device actually exist on Ventana? > Adding it should be a separate patch if so, since this one is just about > introducing the I2C mux, not adding an SBS device. Was this all just > cut/paste from Seaboard without validation? > Yes, I didn't validate this. Actually, I don't know how to validate. I have no idea about SBS device. But I have checked the schematic of Ventana and it has this "bq24617" battery charge controller(for seaboard, it's bq20z75). So I agree with you not to include this part in the patch.
On 10/22/2012 01:29 AM, Mark Zhang wrote: > On 10/19/2012 11:48 PM, Stephen Warren wrote: >> On 10/18/2012 11:58 PM, Mark Zhang wrote: >>> Define pinmux for DDC. The DDC pinmux in Ventana is 2 pins in I2C2. >> >>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts >> >>> - ddc { >>> - nvidia,pins = "ddc", "owc", "spdi", "spdo", >>> - "uac"; >>> - nvidia,function = "rsvd2"; >>> - }; >> >> So that removes the entries for 5 pin groups, yet below, entries are >> only added for the ddc and pta pingroups, so the other 4 pin groups >> become unconfigured. >> > > Right. So I think it should be changed to: > > owc { > nvidia,pins = "owc", "spdi", "spdo", "uac"; > nvidia,function = "rsvd2"; > }; > > Is this right? Looks correct, yes. >>> + >>> + state_i2cmux_ddc: pinmux_i2cmux_ddc { >>> + ddc { >>> + nvidia,pins = "ddc"; >>> + nvidia,function = "i2c2"; >>> + }; >>> + pta { >>> + nvidia,pins = "pta"; >>> + nvidia,function = "rsvd4"; >>> + }; >> >> Does this actually work? The pta pingroup is configured by the "hog" >> pinctrl state of the pinctrl node itself, so this state should fail to >> be applied since it attempts to touch the same pingroup. > > I know little about kernel pinctrl subsystem. After reading some docs > and codes, I think what you mean is, in Ventana's pinmux configuration, > pta pingroup has been defined as "hdmi" function and this can't be > changed(non-dynamic pinmuxing). > So I want to know why we have defined pta pingroup as hdmi function? > Can we remove this definition to make the i2cmux above working? I don't recall why pta was defined to be HDMI. The issue isn't that this patch changes the pinmux selection for the pta pingroup, but simply that both the pinctrl node's state definition, and the new I2C mux node's state definition both attempt to configure pingroup pta. The solution is most likely to simply remove the pta configuration from the main pinctrl node.
On 10/23/2012 01:14 AM, Stephen Warren wrote: > On 10/22/2012 01:29 AM, Mark Zhang wrote: >> On 10/19/2012 11:48 PM, Stephen Warren wrote: >>> On 10/18/2012 11:58 PM, Mark Zhang wrote: >>>> Define pinmux for DDC. The DDC pinmux in Ventana is 2 pins in I2C2. >>> >>>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts >>> >>>> - ddc { >>>> - nvidia,pins = "ddc", "owc", "spdi", "spdo", >>>> - "uac"; >>>> - nvidia,function = "rsvd2"; >>>> - }; >>> >>> So that removes the entries for 5 pin groups, yet below, entries are >>> only added for the ddc and pta pingroups, so the other 4 pin groups >>> become unconfigured. >>> >> >> Right. So I think it should be changed to: >> >> owc { >> nvidia,pins = "owc", "spdi", "spdo", "uac"; >> nvidia,function = "rsvd2"; >> }; >> >> Is this right? > > Looks correct, yes. > >>>> + >>>> + state_i2cmux_ddc: pinmux_i2cmux_ddc { >>>> + ddc { >>>> + nvidia,pins = "ddc"; >>>> + nvidia,function = "i2c2"; >>>> + }; >>>> + pta { >>>> + nvidia,pins = "pta"; >>>> + nvidia,function = "rsvd4"; >>>> + }; >>> >>> Does this actually work? The pta pingroup is configured by the "hog" >>> pinctrl state of the pinctrl node itself, so this state should fail to >>> be applied since it attempts to touch the same pingroup. >> >> I know little about kernel pinctrl subsystem. After reading some docs >> and codes, I think what you mean is, in Ventana's pinmux configuration, >> pta pingroup has been defined as "hdmi" function and this can't be >> changed(non-dynamic pinmuxing). >> So I want to know why we have defined pta pingroup as hdmi function? >> Can we remove this definition to make the i2cmux above working? > > I don't recall why pta was defined to be HDMI. The issue isn't that this > patch changes the pinmux selection for the pta pingroup, but simply that > both the pinctrl node's state definition, and the new I2C mux node's > state definition both attempt to configure pingroup pta. The solution is > most likely to simply remove the pta configuration from the main pinctrl > node. > Understood. I tried to remove the hdmi function definition of pta pingroup yesterday then found HDMI can't work anymore. The EDID of HDMI monitor can't be fetched. After some debugging, I have found that it's caused by "i2c-mux-pinctrl" driver is loaded after drm driver. That makes drm driver can't get EDID data via this i2c mux adapter because it doesn't exist at that time. So I think we need to promote the load level of "i2c-mux-pinctrl" driver. Any other ideas? Mark
On 10/25/2012 12:32 AM, Mark Zhang wrote: > On 10/23/2012 01:14 AM, Stephen Warren wrote: ... >> I don't recall why pta was defined to be HDMI. The issue isn't that this >> patch changes the pinmux selection for the pta pingroup, but simply that >> both the pinctrl node's state definition, and the new I2C mux node's >> state definition both attempt to configure pingroup pta. The solution is >> most likely to simply remove the pta configuration from the main pinctrl >> node. > > Understood. I tried to remove the hdmi function definition of pta > pingroup yesterday then found HDMI can't work anymore. The EDID of HDMI > monitor can't be fetched. After some debugging, I have found that it's > caused by "i2c-mux-pinctrl" driver is loaded after drm driver. That > makes drm driver can't get EDID data via this i2c mux adapter because it > doesn't exist at that time. So I think we need to promote the load level > of "i2c-mux-pinctrl" driver. Any other ideas? (BTW, your message that I'm replying to wasn't word-wrapped correctly) No, i2c-mux-pinctrl shouldn't need to change. It sounds like tegra-drm isn't supporting deferred probe correctly; when it needs access to an I2C adapter that doesn't exist, it should defer its own probe until the I2C adapter does exist.
On 10/25/2012 11:37 PM, Stephen Warren wrote: > On 10/25/2012 12:32 AM, Mark Zhang wrote: >> On 10/23/2012 01:14 AM, Stephen Warren wrote: > ... >>> I don't recall why pta was defined to be HDMI. The issue isn't that this >>> patch changes the pinmux selection for the pta pingroup, but simply that >>> both the pinctrl node's state definition, and the new I2C mux node's >>> state definition both attempt to configure pingroup pta. The solution is >>> most likely to simply remove the pta configuration from the main pinctrl >>> node. >> >> Understood. I tried to remove the hdmi function definition of pta >> pingroup yesterday then found HDMI can't work anymore. The EDID of HDMI >> monitor can't be fetched. After some debugging, I have found that it's >> caused by "i2c-mux-pinctrl" driver is loaded after drm driver. That >> makes drm driver can't get EDID data via this i2c mux adapter because it >> doesn't exist at that time. So I think we need to promote the load level >> of "i2c-mux-pinctrl" driver. Any other ideas? > > (BTW, your message that I'm replying to wasn't word-wrapped correctly) > Yes. I did some settings in my Thunderbird, how about now? > No, i2c-mux-pinctrl shouldn't need to change. It sounds like tegra-drm > isn't supporting deferred probe correctly; when it needs access to an > I2C adapter that doesn't exist, it should defer its own probe until the > I2C adapter does exist. Okay. Yes, tegra-drm doesn't support deferred probe now. I'll talk with Thierry and figure out how to handle this. Thanks. > -- > To unsubscribe from this list: send the line "unsubscribe linux-tegra" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts index bec8bb2..b6b3af4 100644 --- a/arch/arm/boot/dts/tegra20-ventana.dts +++ b/arch/arm/boot/dts/tegra20-ventana.dts @@ -64,11 +64,6 @@ nvidia,pins = "dap4"; nvidia,function = "dap4"; }; - ddc { - nvidia,pins = "ddc", "owc", "spdi", "spdo", - "uac"; - nvidia,function = "rsvd2"; - }; dta { nvidia,pins = "dta", "dtb", "dtc", "dtd", "dte"; nvidia,function = "vi"; @@ -248,6 +243,39 @@ nvidia,slew-rate-falling = <3>; }; }; + + state_i2cmux_ddc: pinmux_i2cmux_ddc { + ddc { + nvidia,pins = "ddc"; + nvidia,function = "i2c2"; + }; + pta { + nvidia,pins = "pta"; + nvidia,function = "rsvd4"; + }; + }; + + state_i2cmux_pta: pinmux_i2cmux_pta { + ddc { + nvidia,pins = "ddc"; + nvidia,function = "rsvd4"; + }; + pta { + nvidia,pins = "pta"; + nvidia,function = "i2c2"; + }; + }; + + state_i2cmux_idle: pinmux_i2cmux_idle { + ddc { + nvidia,pins = "ddc"; + nvidia,function = "rsvd4"; + }; + pta { + nvidia,pins = "pta"; + nvidia,function = "rsvd4"; + }; + }; }; i2s@70002800 { @@ -291,6 +319,38 @@ clock-frequency = <400000>; }; + i2cmux { + compatible = "i2c-mux-pinctrl"; + #address-cells = <1>; + #size-cells = <0>; + + i2c-parent = <&{/i2c@7000c400}>; + + pinctrl-names = "ddc", "pta", "idle"; + pinctrl-0 = <&state_i2cmux_ddc>; + pinctrl-1 = <&state_i2cmux_pta>; + pinctrl-2 = <&state_i2cmux_idle>; + + i2c@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + + smart-battery@b { + compatible = "ti,bq24617", "smart-battery-1.1"; + reg = <0xb>; + ti,i2c-retry-count = <2>; + ti,poll-retry-count = <10>; + }; + }; + }; + i2c@7000c500 { status = "okay"; clock-frequency = <400000>;
Define pinmux for DDC. The DDC pinmux in Ventana is 2 pins in I2C2. Signed-off-by: Mark Zhang <markz@nvidia.com> --- arch/arm/boot/dts/tegra20-ventana.dts | 70 ++++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 5 deletions(-)