[PATCH] arm64: dts: rockchip: Enable pcie2 and audio jack on rk3566-roc-pc

Peter Geis pgwipeout at gmail.com
Sun Oct 9 14:10:32 PDT 2022


On Sun, Oct 9, 2022 at 4:36 PM Heiko Stuebner <heiko at sntech.de> wrote:
>
> Hi Furkan,
>
> Am Sonntag, 9. Oktober 2022, 22:07:07 CEST schrieb Furkan Kardame:
> > Add dts nodes to enable combphy2, pcie2 and audio jack.
> > Disable i2c5 as it not used.
>
> A commit message like this very much indicates that its contents want
> to get split over multiple patches.
>
> A commit should do one thing that can be described with a full commit
> message but not multiple different things.
>
>
> The big patches would be:
> - enable sound on rk3566-roc-pc
> - enable pcie2 on rk3566-roc-pc
>
> plus the other stuff I mention below.
>
>
> > Signed-off-by: Furkan Kardame <f.kardame at manjaro.org>
> > ---
> >  .../arm64/boot/dts/rockchip/rk3566-roc-pc.dts | 97 ++++++++++++++++++-
> >  1 file changed, 93 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-roc-pc.dts b/arch/arm64/boot/dts/rockchip/rk3566-roc-pc.dts
> > index dba648c2f..1e5c83012 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3566-roc-pc.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-roc-pc.dts
> > @@ -39,6 +39,28 @@ hdmi_con_in: endpoint {
> >               };
> >       };
> >
> > +     i2s1_8ch: i2s at fe410000 {
> > +             compatible = "rockchip,rk3568-i2s-tdm";
> > +             reg = <0x0 0xfe410000 0x0 0x1000>;
> > +             interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> > +             assigned-clocks = <&cru CLK_I2S1_8CH_TX_SRC>, <&cru CLK_I2S1_8CH_RX_SRC>;
> > +             assigned-clock-rates = <1188000000>, <1188000000>;
> > +             clocks = <&cru MCLK_I2S1_8CH_TX>, <&cru MCLK_I2S1_8CH_RX>,
> > +                      <&cru HCLK_I2S1_8CH>;
> > +             clock-names = "mclk_tx", "mclk_rx", "hclk";
> > +             dmas = <&dmac1 3>, <&dmac1 2>;
> > +             dma-names = "rx", "tx";
> > +             resets = <&cru SRST_M_I2S1_8CH_TX>, <&cru SRST_M_I2S1_8CH_RX>;
> > +             reset-names = "tx-m", "rx-m";
> > +             rockchip,grf = <&grf>;
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&i2s1m0_sclktx &i2s1m0_sclkrx
> > +                          &i2s1m0_lrcktx &i2s1m0_lrckrx
> > +                          &i2s1m0_sdi0   &i2s1m0_sdo0>;
> > +             #sound-dai-cells = <0>;
> > +             status = "disabled";
> > +     };
>
> this looks like a core i2s controller and should
> therefore be in rk356x.dtsi (as it most likely exists
> in both rk3566 and rk3568) - and be a separate patch.

This already exists in the core, if you just want to override the
pinctrl, all you need is:
&i2s1_8ch {
               pinctrl-0 = <&i2s1m0_sclktx &i2s1m0_sclkrx
                            &i2s1m0_lrcktx &i2s1m0_lrckrx
                            &i2s1m0_sdi0   &i2s1m0_sdo0>;
};


>
>
> >       leds {
> >               compatible = "gpio-leds";
> >
> > @@ -53,6 +75,22 @@ led-user {
> >               };
> >       };
> >
> > +     rk809-sound {
> > +             compatible = "simple-audio-card";
> > +             simple-audio-card,format = "i2s";
> > +             simple-audio-card,name = "STATION-M2-FRONT";
> > +             simple-audio-card,mclk-fs = <256>;
> > +             status = "okay";
> > +
> > +             simple-audio-card,cpu {
> > +                     sound-dai = <&i2s1_8ch>;
> > +             };
> > +
> > +             simple-audio-card,codec {
> > +                     sound-dai = <&rk809>;
> > +             };
> > +     };
> > +
> >       sdio_pwrseq: sdio-pwrseq {
> >               status = "okay";
> >               compatible = "mmc-pwrseq-simple";
> > @@ -82,6 +120,18 @@ vcc5v0_sys: vcc5v0-sys-regulator {
> >               vin-supply = <&usb_5v>;
> >       };
> >
> > +     vcc3v3_pcie: vcc3v3-pcie-regulator {
> > +             compatible = "regulator-fixed";
> > +             enable-active-high;
> > +             gpio = <&gpio0 RK_PC4 GPIO_ACTIVE_HIGH>;
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&pcie_enable_h>;
> > +             regulator-name = "vcc3v3_pcie";
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +             vin-supply = <&vcc5v0_sys>;
> > +     };
> > +
> >       vcc3v3_sys: vcc3v3-sys-regulator {
> >               compatible = "regulator-fixed";
> >               regulator-name = "vcc3v3_sys";
> > @@ -122,6 +172,10 @@ &combphy1 {
> >       status = "okay";
> >  };
> >
> > +&combphy2 {
> > +     status = "okay";
> > +};
> > +
> >  &cpu0 {
> >       cpu-supply = <&vdd_cpu>;
> >  };
> > @@ -142,7 +196,7 @@ &gmac1 {
> >       assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru SCLK_GMAC1>;
> >       assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru SCLK_GMAC1>, <&gmac1_clkin>;
> >       clock_in_out = "input";
> > -     phy-mode = "rgmii-id";
> > +     phy-mode = "rgmii";
>
> what is this doing here?
> Definitly separate and with a lot of explanation
> why it is necessary.

Another failure of rgmii-id, but this should be a separate patch
(preferably first, since it's technically a fix).

>
>
> >       phy-supply = <&vcc_3v3>;
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&gmac1m0_miim
> > @@ -210,9 +264,13 @@ rk809: pmic at 20 {
> >               interrupt-parent = <&gpio0>;
> >               interrupts = <RK_PA7 IRQ_TYPE_LEVEL_LOW>;
> >               clock-output-names = "rk808-clkout1", "rk808-clkout2";
> > -
> > +             assigned-clocks = <&cru I2S1_MCLKOUT_TX>;
> > +             assigned-clock-parents = <&cru CLK_I2S1_8CH_TX>;
> > +             clock-names = "mclk";
> > +             clocks = <&cru I2S1_MCLKOUT_TX>;
> > +             pinctrl-0 = <&pmic_int>, <&i2s1m0_mclk>;
> > +             #sound-dai-cells = <0>;
> >               pinctrl-names = "default";
> > -             pinctrl-0 = <&pmic_int>;
> >               rockchip,system-power-controller;
> >               wakeup-source;
> >               #clock-cells = <1>;
> > @@ -419,6 +477,10 @@ vcc3v3_sd: SWITCH_REG2 {
> >                               regulator-boot-on;
> >                       };
> >               };
> > +
> > +             codec {
> > +                     mic-in-differential;
> > +             };
> >       };
> >  };
> >
> > @@ -432,11 +494,20 @@ &i2c2 {
> >
> >  &i2c3 {
> >       pinctrl-names = "default";
> > -     pinctrl-0 = <&i2c3m1_xfer>;
> > +     pinctrl-0 = <&i2c3m0_xfer>;
>
> Also separate with a paragraph on why.

Again, if this is a correction, it should be a separate fixes patch.

>
>
> >       status = "okay";
> >  };
> >
> >  &i2c5 {
> > +     status = "disabled";
>
> definitly also separate

If this isn't used (as in not available on the hardware) this should
be removed entirely.

>
>
> > +};
> > +
> > +&i2s0_8ch {
> > +     status = "okay";
> > +};
>
> where is this i2s0 connected to?
>
>
> > +
> > +&i2s1_8ch {
> > +     rockchip,trcm-sync-tx-only;
> >       status = "okay";
> >  };
> >
> > @@ -447,6 +518,14 @@ rgmii_phy1: ethernet-phy at 0 {
> >       };
> >  };
> >
> > +&pcie2x1 {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&pcie_reset_h>;
> > +     reset-gpios = <&gpio1 RK_PB2 GPIO_ACTIVE_HIGH>;
> > +     vpcie3v3-supply = <&vcc3v3_pcie>;
> > +     status = "okay";
> > +};
> > +
> >  &pinctrl {
> >       bt {
> >               bt_enable_h: bt-enable-h {
> > @@ -468,6 +547,16 @@ user_led_enable_h: user-led-enable-h {
> >               };
> >       };
> >
> > +     pcie {
> > +             pcie_enable_h: pcie-enable-h {
> > +                     rockchip,pins = <0 RK_PC4 RK_FUNC_GPIO &pcfg_pull_none>;
> > +             };
> > +
> > +             pcie_reset_h: pcie-reset-h {
> > +                     rockchip,pins = <1 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
> > +             };
> > +     };
> > +
> >       pmic {
> >               pmic_int: pmic_int {
> >                       rockchip,pins =
> >
>
>
> Heiko
>
>

Very Respectfully,
Peter



More information about the Linux-rockchip mailing list