Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751703AbdF2APG (ORCPT ); Wed, 28 Jun 2017 20:15:06 -0400 Received: from lucky1.263xmail.com ([211.157.147.135]:45147 "EHLO lucky1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751545AbdF2APF (ORCPT ); Wed, 28 Jun 2017 20:15:05 -0400 X-263anti-spam: X-MAIL-GRAY: 1 X-MAIL-DELIVERY: 0 X-ABS-CHECKED: 4 X-RL-SENDER: shawn.lin@rock-chips.com X-FST-TO: linux-arm-kernel@lists.infradead.org X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: shawn.lin@rock-chips.com X-UNIQUE-TAG: <019d12e5de70d167a170180493b5d786> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Cc: shawn.lin@rock-chips.com, Mark Rutland , devicetree@vger.kernel.org, =?UTF-8?Q?Heiko_St=c3=bcbner?= , Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, kever.yang@rock-chips.com, linux-rockchip@lists.infradead.org, Rob Herring , Philipp Tomsich , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 4/4] arm64: dts: add RK3399-Q7 (Puma) SoM To: Klaus Goger References: <20170626191854.58253-1-klaus.goger@theobroma-systems.com> <20170626191854.58253-5-klaus.goger@theobroma-systems.com> <2336478.rsaXP0O2Er@diego> <8378C275-0F65-4B91-A9B8-23E78799F7E5@theobroma-systems.com> From: Shawn Lin Message-ID: <8319b665-dd0d-36fc-999d-f4ae1afbd38b@rock-chips.com> Date: Thu, 29 Jun 2017 08:14:53 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <8378C275-0F65-4B91-A9B8-23E78799F7E5@theobroma-systems.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3882 Lines: 145 Hi On 2017/6/28 22:01, Klaus Goger wrote: > Hi Shawn, > >> On 28 Jun 2017, at 14:41, Shawn Lin 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@lists.infradead.org >>> http://lists.infradead.org/mailman/listinfo/linux-rockchip >>> >> >> >> -- >> Best Regards >> Shawn Lin > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip >