Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754301AbdLDQIj (ORCPT ); Mon, 4 Dec 2017 11:08:39 -0500 Received: from mail-ua0-f179.google.com ([209.85.217.179]:41202 "EHLO mail-ua0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754370AbdLDQId (ORCPT ); Mon, 4 Dec 2017 11:08:33 -0500 X-Google-Smtp-Source: AGs4zMZtxw65X+71tKPfyq+7gaINsIuxafJwxuEqSFdiluIXJ9pU5wgZWeqxBIKDTNkuAzw5u+MZPFkAUVqbRdK00mc= MIME-Version: 1.0 In-Reply-To: <4091462.xo23GTqF41@diego> References: <1486712654-15431-1-git-send-email-zyw@rock-chips.com> <1941891.vosvJnn3h1@phil> <8042d73f-c1de-da41-9066-2377de1f521c@rock-chips.com> <4091462.xo23GTqF41@diego> From: Doug Anderson Date: Mon, 4 Dec 2017 08:08:31 -0800 X-Google-Sender-Auth: VbePiD8vxqcZO08CroejC090wAA Message-ID: Subject: Re: [PATCH 0/4] Move DP phy switch to PHY driver To: =?UTF-8?Q?Heiko_St=C3=BCbner?= Cc: Chris Zhong , dri-devel@lists.freedesktop.org, Kishon Vijay Abraham I , Rob Herring , "open list:ARM/Rockchip SoC..." , LKML , Guenter Roeck , Sean Paul , William wu , Rob Herring , David Airlie , Shawn Lin , Catalin Marinas , Elaine Zhang , David Wu , Kever Yang , Brian Norris , Tomasz Figa , Will Deacon , devicetree@vger.kernel.org, Linux ARM , Jianqun Xu , Caesar Wang , Mark Rutland , Enric Balletbo i Serra Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id vB4G8mEY022434 Content-Length: 4086 Lines: 96 Hi, On Sun, Dec 3, 2017 at 11:46 PM, Heiko Stübner wrote: > Hi Chris, > > Am Montag, 4. Dezember 2017, 10:47:08 CET schrieb Chris Zhong: >> On 2017年12月02日 05:58, Heiko Stuebner wrote: >> > Am Freitag, 1. Dezember 2017, 13:42:46 CET schrieb Doug Anderson: >> >> Hi, >> >> >> >> On Wed, Nov 29, 2017 at 6:27 PM, Chris Zhong wrote: >> >>> Hi Doug >> >>> >> >>> Thank you for mentioning this patch. >> >>> >> >>> I think the focus of the discussion is: can we put the grf control bit >> >>> to >> >>> dts. >> >>> >> >>> The RK3399 has 2 Type-C phy, but only one DP controller, this >> >>> "uphy_dp_sel" >> >>> >> >>> can help to switch these 2 phy. So I think this bit can be considered as >> >>> a >> >>> part of >> >>> >> >>> Type-C phy, these 2 phy have different bits, just similar to other bits >> >>> (such as "pipe-status"). >> >>> >> >>> Put them to DTS file might be a accepted practice. >> >> >> >> I guess the first step would be finding the person to make a decision. >> >> Is that Heiko? Olof? Kishon? Rob?. As I see it there are a few >> >> options: >> >> >> >> 1. Land this series as-is. This makes the new bit work just like all >> >> the other ones next to it. If anyone happens to try to use an old >> >> device tree on a new kernel they'll break. Seems rather unlikely >> >> given that the whole type C PHY is not really fully functional >> >> upstream, but technically this is a no-no from a device tree >> >> perspective. >> >> >> >> 2. Change the series to make this property optional. If it's not >> >> there then the code behaves like it always did. This would address >> >> the "compatibility" problem but likely wouldn't actually help any real >> >> people, and it would be extra work. >> >> >> >> 3. Redo the driver to deprecate all the old offsets / bits and just >> >> put the table in the driver, keyed off the compatible string and base >> >> address if the IO memory. >> >> >> >> >> >> I can't make this decision. It's up to those folks who would be >> >> landing the patch and I'd be happy with any of them. What I'm less >> >> happy with, however, is the indecision preventing forward progress. >> >> We should pick one of the above things and land it. My own personal >> >> bias is #1: just land the series. No real people will be hurt and >> >> it's just adding another property that matches the ones next to it. >> > >> > I'd second that #1 . That whole type-c phy thingy never fully worked in >> > the past (some for the never used dp output), so personally I don't have >> > issues with going that route. >> > >> >> From a long term perspective (AKA how I'd write the next driver like >> >> >> >> this) I personally lean towards to "tables in the driver, not in the >> >> device tree" but quite honestly I'm happy to take whatever direction >> >> the maintainers give. >> > >> > It looks like we're in agreement here :-) . GRF stuff should not leak into >> > the devicetree, as it causes endless headaches later. But I guess we'll >> > need to live with the ones that happened so far. >> >> So, the first step is: move all the private property of tcphy to >> drivers/phy/rockchip/phy-rockchip-typec.c. >> Second step: new a member: uphy-dp-sel. >> In my mind, we should have discussed these properties before, and then I >> moved them all into DTS. > > Actually, I was agreeing with Doug, that we probably don't need to rework the > type-c phy driver. As most properties for it are in the devicetree right now > we'll need to support them for backwards-compatiblity anyway. > > And yes, there probably was discussion over dts vs. driver-table when the > type-c driver was introduced, but I either missed it or wasn't firm enough > back then ;-) . > > Hence the "we'll need to live with it" for the type-c phy, but should not > do similar things in future drivers. So I guess now we're just waiting for some agreement from Kishon that he's willing to land the PHY change? Heiko: presumably you could apply the DP change to drm-misc? ...or is there some other process needed there? -Doug