Re: [PATCH 4/4] arm64: dts: add RK3399-Q7 (Puma) SoM

[Shawn Lin]

Hi

On 2017/6/28 22:01, Klaus Goger wrote:
> Hi Shawn,
>
> > On 28 Jun 2017, at 14:41, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:

--8<-----------

> > > > +&pcie0 {
> > > > +	ep-gpios = <&gpio4 RK_PC6 GPIO_ACTIVE_LOW>;
> > > > +	num-lanes = <4>;
> > > > +	pinctrl-names = "default";
> > > > +	pinctrl-0 = <&pcie_clkreqn>;
> > > > +	status = "okay";
> > >
> > > vpcie{3v3, 1v8, 0v9}-supply properties?
> >
> > These supply are optional which depend on the HW design.
> > But pcie_clkreqn doesn't really work. I think we should
> > use pcie_clkreqn_cpm instead and fix this for rk3399-evb.dts
> > to prevent the further copy-n-paste.
>
> I could add the supply properties but since they where optional and
> are generated by dedicated always-on regulators supplied from the
> also always on regulator vcc3v3_sys I didnât see the need for it.
> But if anyone to have a full model of the power tree visible in the
> devicetree even if not controllable I can add it in v2.

Ok, so you don't need to add these here. :)

> Since the properties are optional maybe we should also change
> the dev_info "no xxx regulator foundâ in pcie-rockchip.c to something
> less severe sounding.
>
> > > > +};
> > > > +
> > >
> > > [...]
> > >
> > > > +&sdmmc {
> > > > +	clock-frequency = <150000000>;
> >
> > we deprecates this now, so please use max-frequency instead.
> >
> > > > +	bus-width = <4>;
> > > > +	cap-mmc-highspeed;
> > > > +	cap-sd-highspeed;
> > > > +	cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>;
> >
> > Really this board use gpio-based card detect pin?
> >
> > > > +	wp-gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_LOW>;
> > > > +	num-slots = <1>;
> > > > +	pinctrl-names = "default";
> > > > +	pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_gpio &sdmmc_bus4>;
> >
> > I guess you don't need sdmmc_gpio and let mmc core request
> > this gpio as irq pin from parsing cd-gpios?
>
> I tried to just use the PA7 as SDMMC0_DET but didnât get any state changes when

Hmm.. we use PA7 as SDMMC0_DET for vendor kernel 4.4.
I guess something goes wrong here and will check if later.

> removing it or pugging it in after bootup. I tried to follow the mmc code path to see
> which of the 3 card detection modes get configured. It looked to me as the CDETECT
> register gets used but this would not generate any interrupts and requires polling of the
> register. So not using a a gpio card detect requires the broken-cd property for me.
> If i overlooked something Iâm happy to change it, I was planning to take a look at it later
> anyways
>
> > > > +	status = "okay";
> > > > +};
> >
> > And I would be more happy here to see the present of vqmmc and vmmc
> > supply if possible.
>
> Since we are a SoM vmmc would be a property required to be provided from the baseboard
> and not part of the module dts.
> I will add vqmmc for the I/O voltage supplying APIO#

good to know that.

> > > > +
> > > > +
> > >
> > > double empty line
> > >
> > >
> > > > +&spi1 {
> > > > +	status = "okay";
> > > > +
> > > > +	flash: norflash@0 {
> > >
> > > norflash: flash@0 maybe?
> > >
> > > You reference the phandle and at the position it gets referenced
> > > the specific name might be more helpful.
> > >
> > >
> > > > +		compatible = "jedec,spi-nor";
> > > > +		reg = <0>;
> > > > +		spi-max-frequency = <50000000>;
> > > > +	};
> > > > +};
> > >
> > >
> > >
> > > > +&pinctrl {
> > > > +	pinctrl-names = "default";
> > > > +	pinctrl-0 = <&puma_pin_hog>;
> > > > +
> > > > +	hog {
> > > > +			puma_pin_hog: puma_pin_hog {
> > >
> > > puma_pin_hog: puma-pin-hog
> > >
> > > Same for more defined pinctrl nodes below that.
> > >
> > >
> > > Heiko
> > >
> > > _______________________________________________
> > > Linux-rockchip mailing list
> > > Linux-rockchip@xxxxxxxxxxxxxxxxxxx
> > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> >
> >
> > --
> > Best Regards
> > Shawn Lin
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-rockchip