From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751486AbdBUJXJ (ORCPT ); Tue, 21 Feb 2017 04:23:09 -0500 Received: from mail-it0-f50.google.com ([209.85.214.50]:35345 "EHLO mail-it0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751085AbdBUJXB (ORCPT ); Tue, 21 Feb 2017 04:23:01 -0500 MIME-Version: 1.0 In-Reply-To: <2d708416-401a-d2df-5694-9896df47a673@ti.com> References: <1487268932-6469-1-git-send-email-bgolaszewski@baylibre.com> <1487268932-6469-5-git-send-email-bgolaszewski@baylibre.com> <5f194c6d-0937-bac2-31cd-2f1658f68d0d@ti.com> <2d708416-401a-d2df-5694-9896df47a673@ti.com> From: Bartosz Golaszewski Date: Tue, 21 Feb 2017 10:22:59 +0100 Message-ID: Subject: Re: [PATCH 4/4] ARM: dts: da850-evm: add the UI expander node To: Sekhar Nori Cc: David Lechner , Kevin Hilman , Michael Turquette , Patrick Titiano , Laurent Pinchart , Rob Herring , Mark Rutland , Russell King , linux-devicetree , LKML , arm-soc Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2017-02-21 6:03 GMT+01:00 Sekhar Nori : > On Monday 20 February 2017 09:08 PM, Bartosz Golaszewski wrote: >> 2017-02-20 10:36 GMT+01:00 Sekhar Nori : >>> On Thursday 16 February 2017 11:45 PM, Bartosz Golaszewski wrote: >>>> If we're using the UI board and want vpif capture, we need to select >>>> the video capture functionality by driving the sel_c pin low on the >>>> tca6416 expander and sel_a & sel_b pins high. Do it statically by >>>> hogging relevant GPIOs in the device tree. >>>> >>>> Signed-off-by: Bartosz Golaszewski >>>> --- >> >> [snip] >> >>>> >>>> + sel_a { >>>> + gpio-hog; >>>> + gpios = <7 GPIO_ACTIVE_HIGH>; >>>> + output-high; >>>> + line-name = "sel_a"; >>>> + }; >>>> + >>>> + sel_b { >>>> + gpio-hog; >>>> + gpios = <6 GPIO_ACTIVE_HIGH>; >>>> + output-high; >>>> + line-name = "sel_b"; >>>> + }; >>>> + >>>> + sel_c { >>>> + gpio-hog; >>>> + gpios = <5 GPIO_ACTIVE_HIGH>; >>>> + output-low; >>>> + line-name = "sel_c"; >>> >>> I think this is better handled by using an enable-gpios property in vpif >>> capture device-tree node. So in the vpif capture node you would have: >>> >>> enable-gpios = <&tca6416 7 GPIO_ACTIVE_HIGH >>> &tca6416 6 GPIO_ACTIVE_HIGH >>> &tca6416 5 GPIO_ACTIVE_LOW>; >>> >>> and in the vpif capture driver, you would request each of these gpios >>> using: devm_gpiod_get_array_optional(.., .., GPIOD_OUT_HIGH); >>> >> >> I'm not sure about this one - the result is the same (function still >> defined statically in the DT) while now it requires changes to the >> vpif driver too. >> >> Is there any other reason we'd prefer this approach? > > The GPIO hog functionality can race with driver probe. Based on probe > order, you may have a situation where VPIF probes before tca6416 so we > have an erroneous situation where probe is successful, but hardware is > not really available. > > Using enable-gpios lets you handle probe deferral so VPIF capture probe > completes only when hardware is ready. So if for some reason tca6416 > driver or hardware is misbehaving, VPIF will know about it through some > error value rather than just assuming that everything went well. > > So, yes, in the "all goes well" scenario, there is not much difference > in the two approaches. But the difference will be apparent when > something goes wrong. > > Probe order will also influence the shutdown and suspend order. So > kernel will automatically make sure that shutdown happens in reverse > probe order. This may or may not matter in this case. But in general, > it will be nice to make sure VPIF shuts down before tca6416 does so that > hardware is available for VPIF to cleanly shutdown (and not disconnected > behind its back because tca6416 decided to put all its lines to off as > part of its shutdown). > > I think GPIO hog should only be used for pins which are really "system > level". IOW, not related to any driver functionality. > > Thanks, > Sekhar Ok, I'll extend the driver then. Thanks, Bartosz