Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To:     Philippe CORNU <philippe.cornu@st.com>
Cc:     Rob Herring <robh@kernel.org>,
        Archit Taneja <architt@codeaurora.org>,
        Andrzej Hajda <a.hajda@samsung.com>,
        David Airlie <airlied@linux.ie>,
        Mark Rutland <mark.rutland@arm.com>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Yannick FERTRE <yannick.fertre@st.com>,
        Benjamin Gaignard <benjamin.gaignard@linaro.org>,
        Alexandre TORGUE <alexandre.torgue@st.com>
Subject: Re: [PATCH v2 1/2] dt-bindings/display/bridge: sii902x: add optional power supplies
Date:   Mon, 14 May 2018 20:06:38 +0300
Message-ID: <1617167.qDJKxXFJBi@avalon>
Organization: Ideas on Board Oy
In-Reply-To: <7fe8e766-56f5-38a3-b4dd-b44d4a5bdcf8@st.com>
References: <20180425075314.19137-1-philippe.cornu@st.com> <3518575.OAoTTXDygq@avalon> <7fe8e766-56f5-38a3-b4dd-b44d4a5bdcf8@st.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Philippe,

On Monday, 14 May 2018 12:22:16 EEST Philippe CORNU wrote:
> On 04/26/2018 12:05 AM, Laurent Pinchart wrote:
> > On Wednesday, 25 April 2018 20:11:23 EEST Rob Herring wrote:
> >> On Wed, Apr 25, 2018 at 04:17:25PM +0300, Laurent Pinchart wrote:
> >>> On Wednesday, 25 April 2018 15:20:04 EEST Philippe CORNU wrote:
> >>>> On 04/25/2018 11:01 AM, Laurent Pinchart wrote:
> >>>>> On Wednesday, 25 April 2018 10:53:13 EEST Philippe Cornu wrote:
> >>>>>> Add optional power supplies using the description found in
> >>>>>> "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)".
> >>>>>>
> >>>>>> There is a single 1v2 supply voltage named vcc12 from which cvcc12
> >>>>>> (digital core) and avcc12 (TMDS analog) are derived because
> >>>>>> according to this data sheet:
> >>>>>> "cvcc12 and avcc12 can be derived from the same power source"
> >>>>>
> >>>>> Shouldn't the power supplies be mandatory, as explained by Mark in
> >>>>> https://lists.freedesktop.org/archives/dri-devel/2018-April/
> >>>>> 172400.html ?
> >>>>
> >>>> Laurent,
> >>>> Many thanks Laurent for your comment, I understood the merge of the
> >>>> two 1v2 power supplies but missed the "mandatory" part... maybe because
> >>>> this patch (with optional power supplies) already got the reviewed-by
> >>>> from Rob, I thought the discussion thread you pointed out was
> >>>> applicable "only" to totally new driver documentation.
> >>>>
> >>>> So, on my side, as a "new user" of sii902x IC, no problem to put these
> >>>> power supplies as mandatory instead of optional properties but I would
> >>>> like to be sure this is applicable to both old and new bindings doc :
> >>>> )
> >>>
> >>> We obviously need to retain backward compatibility, so on the driver
> >>> side you need to treat those power supplies as optional. From a DT
> >>> bindings point of view, however, I think they should be mandatory for
> >>> new DT.
> >>
> >> We don't really have a way to describe these 3 conditions (required for
> >> all, optional for all, and required for new). So generally we make
> >> additions optional. The exception sometimes is if we update all the dts
> >> files.
> > 
> > Can't we just make it mandatory in the bindings, as long as we treat it
> > as optional in drivers ?
> 
> How to progress on this patch? Do you have any suggestions?

By seeing what Rob thinks about my proposal above ? :-)

> >>>> Rob,
> >>>> could you please confirm these power supply properties should be
> >>>> "mandatory"? if yes, should we then modify other optional properties
> >>>> like the reset-gpios too in the future?
> >>>
> >>> The GPIOs properties are different in my opinion, as there's no
> >>> requirement to connect for instance the reset pin to a GPIO controllable
> >>> by the SoC. The pin could be hardwired to VCC, or connected to a system
> >>> reset that is automatically managed without SoC intervention. The power
> >>> supplies, however, are mandatory, in the sense that the chip will not
> >>> work if you leave the power supplies unconnected.
> >>
> >> DT only needs to describe what matters to s/w. If a regulator is
> >> fixed and you don't need to know its voltage (or other read-only
> >> parameters), then there's not much point in putting it in DT.
> >>
> >> I'd probably base this more at a platform level and you either use
> >> regulator binding or you don't. It's perfectly valid that you want to do
> >> things like regulator setup, pin ctrl and muxing setup, etc. all in
> >> firmware and the OS doesn't touch any of that.
> >>
> >> That's all a big can of worms which we shouldn't solve on this 2 line
> >> change. I think this change is fine as-is, so:
> >>
> >> Reviewed-by: Rob Herring <robh@kernel.org>

-- 
Regards,

Laurent Pinchart