On Thu, Aug 03, 2017 at 09:45:23AM +0200, Maciej Purski wrote:
> This patch adds HDMI and Sil9234 MHL converter to Trats2 board.
Just "Add HDMI...", without this patch.
Except few minor nitpicks below, looks good. After fixing I will take it
once bindings got accepted.
>
> Based on previous work by:
> Tomasz Stanislawski <t.stanislaws@xxxxxxxxxxx>
>
> Signed-off-by: Maciej Purski <m.purski@xxxxxxxxxxx>
> ---
> arch/arm/boot/dts/exynos4412-trats2.dts | 93 +++++++++++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
> index 35e9b94..39940f6 100644
> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
> @@ -97,6 +97,16 @@
> gpio = <&gpj0 5 GPIO_ACTIVE_HIGH>;
> enable-active-high;
> };
> +
> + vsil: voltage-regulator-vsil {
> + compatible = "regulator-fixed";
> + regulator-name = "HDMI_5V";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + gpio = <&gpl0 4 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + vin-supply = <&buck7_reg>;
I think the supply is V_BAT, not buck7.
> + };
> };
>
> gpio-keys {
> @@ -229,6 +239,34 @@
> };
> };
>
> + i2c-mhl {
> + compatible = "i2c-gpio";
> + gpios = <&gpf0 4 GPIO_ACTIVE_HIGH &gpf0 6 GPIO_ACTIVE_HIGH>;
> + i2c-gpio,delay-us = <100>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pinctrl-0 = <&i2c_mhl_bus>;
> + pinctrl-names = "default";
> + status = "okay";
> +
> + sii9234: sii9234@39 {
The name of node should be rather generic (ePAPR: "The name of a node
should be somewhat generic, reflecting the function of the device and
not its precise programming model").
So maybe "sii9234: hdmi-bridge@39"?
> + compatible = "sil,sii9234";
> + vcc-supply = <&vsil>;
> + reset-gpios = <&gpf3 4 GPIO_ACTIVE_HIGH>;
> + interrupt-parent = <&gpf3>;
> + interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0x39>;
> +
> +
One empty line instead of two.
> + port {
> + mhl_to_hdmi: endpoint {
> + remote-endpoint = <&hdmi_to_mhl>;
> + };
> + };
> + };
> + };
> +
> camera: camera {
> pinctrl-0 = <&cam_port_a_clk_active &cam_port_b_clk_active>;
> pinctrl-names = "default";
> @@ -522,6 +560,31 @@
> status = "okay";
> };
>
> +&hdmi {
> + hpd-gpios = <&gpx3 7 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_hpd>;
> + hdmi-en-supply = <&vsil>;
> + vdd-supply = <&ldo3_reg>;
> + vdd_osc-supply = <&ldo4_reg>;
> + vdd_pll-supply = <&ldo3_reg>;
> + ddc = <&i2c_5>;
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@1 {
> + reg = <1>;
> + hdmi_to_mhl: endpoint {
> + remote-endpoint = <&mhl_to_hdmi>;
> + };
> + };
> + };
> +
Unnecessary empty line.
> +};
> +
> &hsotg {
> vusb_d-supply = <&ldo15_reg>;
> vusb_a-supply = <&ldo12_reg>;
> @@ -600,6 +663,11 @@
> };
> };
>
> +
> +&i2c_5 {
> + status = "okay";
> +};
Could you describe more what is on i2c_5 and i2c_8? Is it relevant to
this patch?
> +
> &i2c_7 {
> samsung,i2c-sda-delay = <100>;
> samsung,i2c-slave-addr = <0x10>;
> @@ -894,12 +962,20 @@
> };
> };
>
> +&i2c_8 {
> + status = "okay";
> +};
> +
> &i2s0 {
> pinctrl-0 = <&i2s0_bus>;
> pinctrl-names = "default";
> status = "okay";
> };
>
> +&mixer {
> + status = "okay";
> +};
> +
> &mshc_0 {
> num-slots = <1>;
> broken-cd;
> @@ -926,6 +1002,18 @@
> pinctrl-names = "default";
> pinctrl-0 = <&sleep0>;
>
> + mhl_int: mhl-int {
> + samsung,pins = "gpf3-5";
> + samsung,pin-pud = <0>;
Please use defines from dt-bindings/pinctrl/samsung.h
> + };
> +
> + i2c_mhl_bus: i2c-mhl-bus {
> + samsung,pins = "gpf0-4", "gpf0-6";
> + samsung,pin-function = <2>;
> + samsung,pin-pud = <1>;
> + samsung,pin-drv = <0>;
The same.
Best regards,
Krzysztof
--
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
