Hi
On 2017/6/28 22:01, Klaus Goger wrote:
> Hi Shawn,
>
>> On 28 Jun 2017, at 14:41, Shawn Lin <shawn.lin at 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 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.
>>>
>>>
>>>> + 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 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
>
