| Message ID | 20170126132315.17955-4-romain.perier@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Romain, Am Donnerstag, 26. Januar 2017, 14:23:15 CET schrieb Romain Perier: > This commit adds the DT definition of the es8388 i2c device > found at address 0x10. It also adds the definition for connecting > the Rockchip I2S to the es8388 analog output. > > This commit is based on the initial work that was done by Sjoerd Simons > <sjoerd.simons@collabora.com> with some improvements. > > Signed-off-by: Romain Perier <romain.perier@collabora.com> > --- > > Changes in v6: None > Changes in v5: None > Changes in v4: > - Updated to the new DT binding > - Added the property 'rockchip,routing' > - Renamed the node sound_es8388 to sound_i2s > Changes in v3: None > Changes in v2: None > > arch/arm/boot/dts/rk3288-rock2-square.dts | 39 > +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) > > diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts > b/arch/arm/boot/dts/rk3288-rock2-square.dts index dd3ad2e..6b176b8 100644 > --- a/arch/arm/boot/dts/rk3288-rock2-square.dts > +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts > @@ -86,6 +86,19 @@ > #sound-dai-cells = <0>; > }; > > + sound_i2s { > + compatible = "rockchip,rk3288-hdmi-analog"; > + rockchip,model = "I2S"; Are you sure "I2S" is not to generic? Don't know enough about sound, but remember this somehow matching against possible alsa ucm profiles. So this could maybe produce conflicts with such a generic name? > + rockchip,i2s-controller = <&i2s>; > + rockchip,audio-codec = <&es8388>; > + rockchip,routing = "Analog", "LOUT2", > + "Analog", "ROUT2"; > + rockchip,hp-en-gpios = <&gpio8 0 GPIO_ACTIVE_HIGH>; > + rockchip,hp-det-gpios = <&gpio7 7 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&headphone>; > + }; > + > sdio_pwrseq: sdio-pwrseq { > compatible = "mmc-pwrseq-simple"; > clocks = <&hym8563>; > @@ -173,10 +186,29 @@ > }; > }; > > +&i2c2 { > + status = "okay"; > + > + es8388: es8388@10 { > + compatible = "everest,es8388", "everest,es8328"; > + reg = <0x10>; > + AVDD-supply = <&vcca_codec>; > + DVDD-supply = <&vcca_codec>; According to the schematics I have, this comes from vccio_codec and thus from vcc_io So please give the regulator node simply a second phandle, like vcc_io: vccio_codec: REG2 { and reference the correct regulator here. See DCDC_REG4 in rk3288-veyron.dtsi for reference. > + HPVDD-supply = <&vcca_codec>; > + PVDD-supply = <&vcca_codec>; vccio_codec as well > + clocks = <&cru SCLK_I2S0_OUT>; > + clock-names = "i2s_clk_out"; > + }; > +}; > + > &i2c5 { > status = "okay"; > }; > > +&i2s { > + status = "okay"; > +}; > + > &pinctrl { > ir { > ir_int: ir-int { > @@ -190,6 +222,13 @@ > }; > }; > > + sound { > + headphone: headphone { > + rockchip,pins = <8 0 RK_FUNC_GPIO &pcfg_pull_up>, > + <7 7 RK_FUNC_GPIO &pcfg_pull_none>; please use real pin names from schematics fro pinctrl entries when they are known. This makes greping for things easier. So please two separate definitions, "hp_det" and "phone_ctl". Heiko
Hi Heiko, Le 31/01/2017 à 14:23, Heiko Stuebner a écrit : > Hi Romain, > > Am Donnerstag, 26. Januar 2017, 14:23:15 CET schrieb Romain Perier: >> This commit adds the DT definition of the es8388 i2c device >> found at address 0x10. It also adds the definition for connecting >> the Rockchip I2S to the es8388 analog output. >> >> This commit is based on the initial work that was done by Sjoerd Simons >> <sjoerd.simons@collabora.com> with some improvements. >> >> Signed-off-by: Romain Perier <romain.perier@collabora.com> >> --- >> >> Changes in v6: None >> Changes in v5: None >> Changes in v4: >> - Updated to the new DT binding >> - Added the property 'rockchip,routing' >> - Renamed the node sound_es8388 to sound_i2s >> Changes in v3: None >> Changes in v2: None >> >> arch/arm/boot/dts/rk3288-rock2-square.dts | 39 >> +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) >> >> diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts >> b/arch/arm/boot/dts/rk3288-rock2-square.dts index dd3ad2e..6b176b8 100644 >> --- a/arch/arm/boot/dts/rk3288-rock2-square.dts >> +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts >> @@ -86,6 +86,19 @@ >> #sound-dai-cells = <0>; >> }; >> >> + sound_i2s { >> + compatible = "rockchip,rk3288-hdmi-analog"; >> + rockchip,model = "I2S"; > Are you sure "I2S" is not to generic? Don't know enough about sound, but > remember this somehow matching against possible alsa ucm profiles. > > So this could maybe produce conflicts with such a generic name? From what I understood this is the name of the card that will be shown by Alsa, including these displayed by aplay -L . The problem is this driver will connect a cpu dai to multicodecs, one of these is an analog output while the other one is a digital output... I am not sure that we can really expose a different name. > > >> + rockchip,i2s-controller = <&i2s>; >> + rockchip,audio-codec = <&es8388>; >> + rockchip,routing = "Analog", "LOUT2", >> + "Analog", "ROUT2"; >> + rockchip,hp-en-gpios = <&gpio8 0 GPIO_ACTIVE_HIGH>; >> + rockchip,hp-det-gpios = <&gpio7 7 GPIO_ACTIVE_HIGH>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&headphone>; >> + }; >> + >> sdio_pwrseq: sdio-pwrseq { >> compatible = "mmc-pwrseq-simple"; >> clocks = <&hym8563>; >> @@ -173,10 +186,29 @@ >> }; >> }; >> >> +&i2c2 { >> + status = "okay"; >> + >> + es8388: es8388@10 { >> + compatible = "everest,es8388", "everest,es8328"; >> + reg = <0x10>; >> + AVDD-supply = <&vcca_codec>; >> + DVDD-supply = <&vcca_codec>; > According to the schematics I have, this comes from > vccio_codec > and thus from vcc_io > > So please give the regulator node simply a second phandle, like > vcc_io: vccio_codec: REG2 { Ok. > > and reference the correct regulator here. > See DCDC_REG4 in rk3288-veyron.dtsi for reference. > > >> + HPVDD-supply = <&vcca_codec>; >> + PVDD-supply = <&vcca_codec>; > vccio_codec as well OK > > >> + clocks = <&cru SCLK_I2S0_OUT>; >> + clock-names = "i2s_clk_out"; >> + }; >> +}; >> + >> &i2c5 { >> status = "okay"; >> }; >> >> +&i2s { >> + status = "okay"; >> +}; >> + >> &pinctrl { >> ir { >> ir_int: ir-int { >> @@ -190,6 +222,13 @@ >> }; >> }; >> >> + sound { >> + headphone: headphone { >> + rockchip,pins = <8 0 RK_FUNC_GPIO &pcfg_pull_up>, >> + <7 7 RK_FUNC_GPIO &pcfg_pull_none>; > please use real pin names from schematics fro pinctrl entries when they are > known. This makes greping for things easier. > > So please two separate definitions, "hp_det" and "phone_ctl". Ok, I will change that. Thanks, Romain
diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts b/arch/arm/boot/dts/rk3288-rock2-square.dts index dd3ad2e..6b176b8 100644 --- a/arch/arm/boot/dts/rk3288-rock2-square.dts +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts @@ -86,6 +86,19 @@ #sound-dai-cells = <0>; }; + sound_i2s { + compatible = "rockchip,rk3288-hdmi-analog"; + rockchip,model = "I2S"; + rockchip,i2s-controller = <&i2s>; + rockchip,audio-codec = <&es8388>; + rockchip,routing = "Analog", "LOUT2", + "Analog", "ROUT2"; + rockchip,hp-en-gpios = <&gpio8 0 GPIO_ACTIVE_HIGH>; + rockchip,hp-det-gpios = <&gpio7 7 GPIO_ACTIVE_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&headphone>; + }; + sdio_pwrseq: sdio-pwrseq { compatible = "mmc-pwrseq-simple"; clocks = <&hym8563>; @@ -173,10 +186,29 @@ }; }; +&i2c2 { + status = "okay"; + + es8388: es8388@10 { + compatible = "everest,es8388", "everest,es8328"; + reg = <0x10>; + AVDD-supply = <&vcca_codec>; + DVDD-supply = <&vcca_codec>; + HPVDD-supply = <&vcca_codec>; + PVDD-supply = <&vcca_codec>; + clocks = <&cru SCLK_I2S0_OUT>; + clock-names = "i2s_clk_out"; + }; +}; + &i2c5 { status = "okay"; }; +&i2s { + status = "okay"; +}; + &pinctrl { ir { ir_int: ir-int { @@ -190,6 +222,13 @@ }; }; + sound { + headphone: headphone { + rockchip,pins = <8 0 RK_FUNC_GPIO &pcfg_pull_up>, + <7 7 RK_FUNC_GPIO &pcfg_pull_none>; + }; + }; + usb { host_vbus_drv: host-vbus-drv { rockchip,pins = <0 14 RK_FUNC_GPIO &pcfg_pull_none>;
This commit adds the DT definition of the es8388 i2c device found at address 0x10. It also adds the definition for connecting the Rockchip I2S to the es8388 analog output. This commit is based on the initial work that was done by Sjoerd Simons <sjoerd.simons@collabora.com> with some improvements. Signed-off-by: Romain Perier <romain.perier@collabora.com> --- Changes in v6: None Changes in v5: None Changes in v4: - Updated to the new DT binding - Added the property 'rockchip,routing' - Renamed the node sound_es8388 to sound_i2s Changes in v3: None Changes in v2: None arch/arm/boot/dts/rk3288-rock2-square.dts | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)