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@xxxxxxxxxxxxx> with some improvements.
>>
>> Signed-off-by: Romain Perier <romain.perier@xxxxxxxxxxxxx>
>> ---
>>
>> 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
--
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
