Hi Nishanth,
thanks for reviewing this patch. Please see my comments below.
On Wed, Jan 22, 2014 at 01:48:14PM -0600, Nishanth Menon wrote:
> On 01/22/2014 01:04 PM, Christoph Fritz wrote:
> > + regulators {
> > + compatible = "simple-bus";
> Shrug, just moving the fixed regulator to root also will do the job,
> not sure simple-bus much sense here :(
>
will get fixed
> > + reg_vcc3: vcc3 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "VCC3";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-always-on;
> > + };
> > + };
> > +
> > + hsusb1_phy: hsusb1_phy {
> > + compatible = "usb-nop-xceiv";
> > + vcc-supply = <®_vcc3>;
> > + };
> > +};
> > +
> > +&omap3_pmx_wkup {
> > + pinctrl-names = "default";
> > +
> > + lan9221_pins: pinmux_lan9221_pins {
> > + pinctrl-single,pins = <
> > + 0x5A (PIN_INPUT | MUX_MODE4) /* gpio_129 */
> umm.. you might want to follow the convention as you followed later in
> comments.
My convention was: If pin can be described unique with one name,
don't use two. But I'll change that to be uniform with all comments.
> > + >;
> > + };
> > +
> > + tsc2048_pins: pinmux_tsc2048_pins {
> > + pinctrl-single,pins = <
> > + 0x16 (PIN_INPUT_PULLUP | MUX_MODE4) /* gpio_8 */
> umm.. you might want to follow the convention as you followed later in
> comments.
will get fixed
> > + >;
> > + };
> > +
> > + mmc1cd_pins: pinmux_mmc1cd_pins {
> > + pinctrl-single,pins = <
> > + 0x56 (PIN_INPUT | MUX_MODE4) /* gpio_126 */
> umm.. you might want to follow the convention as you followed later in
> comments.
will get fixed
> > + >;
> > + };
> > +};
> > +
> > +&omap3_pmx_core {
> > + pinctrl-names = "default";
> > +
> > + gpio1_pins: pinmux_gpio1_pins {
> > + pinctrl-single,pins = <
> > + 0x5ca (PIN_OUTPUT_PULLDOWN | MUX_MODE4) /* gpio_29 */
> umm.. you might want to follow the convention as you followed later in
> comments.
will get fixed
> > + >;
> > + };
> > +
> > + uart1_pins: pinmux_uart1_pins {
> > + pinctrl-single,pins = <
> > + 0x14c (PIN_OUTPUT | MUX_MODE0) /* uart1_tx.uart1_tx */
> > + 0x14e (PIN_OUTPUT | MUX_MODE0) /* uart1_rts.uart1_rts */
> > + 0x150 (PIN_INPUT | MUX_MODE0) /* uart1_cts.uart1_cts */
> > + 0x152 (PIN_INPUT | MUX_MODE0) /* uart1_rx.uart1_rx */
> > + >;
> you may be interested in include/dt-bindings/pinctrl/omap.h
> OMAP3_CORE1_IOPAD, OMAP3_CORE2_IOPAD as needed here.
will get fixed
> > + };
> > +
> > + uart2_pins: pinmux_uart2_pins {
> > + pinctrl-single,pins = <
> > + 0x140 (PIN_OUTPUT | MUX_MODE1) /* mcbsp3_clkx.uart2_tx */
> > + 0x142 (PIN_INPUT | MUX_MODE1) /* mcbsp3_fsx.uart2_rx */
> > + >;
> > + };
> > +
> > + uart3_pins: pinmux_uart3_pins {
> > + pinctrl-single,pins = <
> > + 0x16e (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
> > + 0x170 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
> > + >;
> > + };
> > +
> > + i2c1_pins: pinmux_i2c1_pins {
> > + pinctrl-single,pins = <
> > + 0x18a (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c1_scl */
> > + 0x18c (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c1_sda */
> umm.. you might want to follow the convention as you followed
> elsewhere in comments.
will get fixed
> > + >;
> > + };
> > +
> > + i2c2_pins: pinmux_i2c2_pins {
> > + pinctrl-single,pins = <
> > + 0x18e (PIN_INPUT | MUX_MODE0) /* i2c2_scl.i2c2_scl */
> > + 0x190 (PIN_INPUT | MUX_MODE0) /* i2c2_sda.i2c2_sda */
> > + >;
> > + };
> > +
> > + i2c3_pins: pinmux_i2c3_pins {
> > + pinctrl-single,pins = <
> > + 0x192 (PIN_INPUT | MUX_MODE0) /* i2c3_scl.i2c3_scl */
> > + 0x194 (PIN_INPUT | MUX_MODE0) /* i2c3_sda.i2c3_sda */
> > + >;
> > + };
> > +
> > + hsusb1_pins: pinmux_hsusb1_pins {
> > + pinctrl-single,pins = <
> > + 0x5a8 (PIN_OUTPUT | MUX_MODE3) /* etk_clk.hsusb1_stp */
> &omap3_pmx_core2 and OMAP3_CORE2_IOPAD probably here. and probably see
> similar usage in other board dtsi.
will get fixed
> > +
> > +#include "twl4030.dtsi"
> > +#include "twl4030_omap3.dtsi"
> > +
> > +&twl {
> > + vmmc1: regulator-vmmc1 {
> > + regulator-always-on;
> > + };
> > +
> > + vdd1: regulator-vdd1 {
> > + regulator-always-on;
> > + };
> > +
> > + vdd2: regulator-vdd2 {
> > + regulator-always-on;
> > + };
> I hope you have covered all required regulators here including the
> ones you might need for IO.1P8 perhaps?
Yes, I rechecked all and it's fine now. IO.1P8 was low due to a
hardware error which is now fixed.
> > +
> > +&mmc1 {
> > + reg = <0x4809c000 0x400>;
> little curious as to why this. is that to override length 0x200 to
> 0x400? that belongs to soc.dtsi then.
This is a remnant from last year and will get purged.
Let me reroll this patch in a new v3 set.
Thanks
-- Christoph
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
