Hi

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

Hi Shawn,

quoted

On 28 Jun 2017, at 14:41, Shawn Lin [off-list ref] wrote:

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

quoted

quoted

quoted

+&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.

quoted

quoted

quoted

+};
+

[...]

quoted

+&sdmmc {
+	clock-frequency = <150000000>;

we deprecates this now, so please use max-frequency instead.

quoted

quoted

+	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?

quoted

quoted

+	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

quoted

quoted

quoted

+	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.

quoted

quoted

quoted

+
+

double empty line

quoted

+&spi1 {
+	status = "okay";
+
+	flash: norflash at 0 {

norflash: flash at 0 maybe?

You reference the phandle and at the position it gets referenced
the specific name might be more helpful.

quoted

+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <50000000>;
+	};
+};

quoted

+&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 at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

-- 
Best Regards
Shawn Lin

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