Am Donnerstag, 6. Juni 2024, 15:13:20 CEST schrieb Space Meyer:
> + Sebastian Reichel regarding pcie3x4 BAR 1 overlap
>
> On 02.06.2024 22:20, Sebastian Kropatsch wrote:
> > Some RK3588 boards are still using this property, the following quote
> > is from rk3588-tiger-haikou.dts for example:
> > &sdmmc {
> > /* while the same pin, sdmmc_det does not detect card changes */
> > cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>;
> >
> > I am unsure as to whether this comment from the quote might apply for
> > the CM3588 as well. Please let me know if you are able to tell :-)
>
> I don't quite understand this. However GPIO0_A4 *is* routed to the micro
> sd CD according to the NAS schematic, page 16 around A5.
for the actual sdmmc_det functionality ... possibly some pinconfig thing?
I.e. pull-whatever settings?
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588-nas.dts
>
> > + adc_keys: adc-keys {
>
> AFAICT this board uses only 1 button per ADC input. Hence I think we
> need seperate ADC defs per button. The usual plural "adc-keys" does not
> apply.
Also if you need to define multiple ones, you'll need distinctive names.
> > + analog-sound {
> > + compatible = "simple-audio-card";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&headphone_detect>;
> > +
> > + simple-audio-card,name = "realtek,rt5616-codec";
> > + simple-audio-card,format = "i2s";
> > + simple-audio-card,mclk-fs = <256>;
> > +
> > + simple-audio-card,hp-det-gpio = <&gpio1 RK_PC4 GPIO_ACTIVE_LOW>;
> > +
> > + simple-audio-card,routing =
> > + "Headphones", "HPOL",
> > + "Headphones", "HPOR",
> > + "MIC1", "Microphone Jack",
> > + "Microphone Jack", "micbias1";
> > + simple-audio-card,widgets =
> > + "Headphone", "Headphones",
> > + "Microphone", "Microphone Jack";
> > +
> > + simple-audio-card,cpu {
> > + sound-dai = <&i2s0_8ch>;
> > + };
> > +
> > + simple-audio-card,codec {
> > + sound-dai = <&rt5616>;
> > + };
> > + };
>
> The rt5616 is on the SoM according to the schematic. Maybe move it all
> there and then only define the hp-det-gpio here?
When the codec itself is on the som _it_ should be there.
For the card itself I don't necessarily think so.
I.e. another baseboard (if it ever exists) might not route sound at all,
so I guess it might make more sense to leave the actual "card" on the
baseboard that also provides the actual outputs and inputs.
> > + vcc_3v3_host_32: regulator-vcc-3v3-host-32 {
> > + compatible = "regulator-fixed";
> > + enable-active-high;
> > + gpios = <&gpio3 RK_PA5 GPIO_ACTIVE_HIGH>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&vcc_3v3_host32_en>;
> > + regulator-name = "vcc_3v3_host_32";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + vin-supply = <&vcc_5v0_sys>;
> > + };
>
> I think this is a 5v0 regulator?
At least the vcc_3v3_host_32 naming suggests 3.3V though?
> > + vcc_3v3_pcie30: regulator-vcc-3v3-pcie30 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "vcc_3v3_pcie30";
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + vin-supply = <&vcc_5v0_sys>;
> > + };
>
> These are 4 seperate regulators according to the schematic. However, as
> they are all fixed, idk if they should be split or kept like this.
personally, I really like the power-diagram to match schematics.
I.e. $debugfs/regulator/regulator_summary will produce a really nice
graph of all the system's regulators, so it's definitly nice if the
hirarchy matches. Also prevents head-scratching later on ;-)
> > + fusb302: typec-portc@22 {
> > + compatible = "fcs,fusb302";
> > + reg = <0x22>;
> > + interrupt-parent = <&gpio0>;
> > + interrupts = <RK_PD3 IRQ_TYPE_LEVEL_LOW>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&usbc0_int>;
> > + vbus-supply = <&vbus_5v0_typec>;
>
> Isn't this missing a `status = "okay";`?
status okay is the default, so when you add a completely new node it is
"okay" by default and you only add a status if you need something else.
> > +&pinctrl {
> > + audio {
> > + headphone_detect: headphone-detect {
> > + rockchip,pins = <1 RK_PC4 RK_FUNC_GPIO &pcfg_pull_none>;
>
> You could use &gpio1 instead of 1. Same for every entry in &pinctrl.
No, that is a number - that of the pin-bank and not a reference to the
gpio controller. Please any other board/soc with its pin-groups.
> > +&pinctrl {
> > + gpio-leds {
> > + led_sys_pin: led-sys-pin {
> > + rockchip,pins = <2 RK_PC5 RK_FUNC_GPIO &pcfg_pull_none>;
>
> You could use &gpio2 instead of 2. Same for every entry in &pinctrl.
same as above
Heiko
