Hi

On 2017/6/28 22:01, Klaus Goger wrote:
Hi Shawn,

On 28 Jun 2017, at 14:41, Shawn Lin <shawn....@rock-chips.com> 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-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip



--
Best Regards
Shawn Lin


_______________________________________________
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip


Reply via email to