public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>, Sakari Ailus <sakari.ailus@iki.fi>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-media <linux-media@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [PATCH v2 2/4] [media] ad5820: Add support for enable pin
Date: Thu, 20 Sep 2018 23:04:21 +0300	[thread overview]
Message-ID: <1887575.If169ivQRG@avalon> (raw)
In-Reply-To: <CAPybu_2mNE7Jmfm2n60Z9Hk_iO+-zLgtu4xn72pJUSXBitVg=g@mail.gmail.com>

Hi Ricardo,

On Thursday, 20 September 2018 22:12:44 EEST Ricardo Ribalda Delgado wrote:
> On Thu, Sep 20, 2018 at 9:08 PM Pavel Machek <pavel@ucw.cz> wrote:
> > On Thu 2018-09-20 21:06:16, Ricardo Ribalda Delgado wrote:
> >> On Thu, Sep 20, 2018 at 8:54 PM Pavel Machek <pavel@ucw.cz> wrote:
> >>> On Thu 2018-09-20 20:45:52, Ricardo Ribalda Delgado wrote:
> >>>> This patch adds support for a programmable enable pin. It can be
> >>>> used in situations where the ANA-vcc is not configurable (dummy-
> >>>> regulator), or just to have a more fine control of the power saving.
> >>>> 
> >>>> The use of the enable pin is optional.
> >>>> 
> >>>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> >>> 
> >>> Do we really want to do that?
> >>> 
> >>> Would it make more sense to add gpio-regulator and connect ad5820 to
> >>> it in such case?
> >> 
> >> My board (based on db820c)  has both:
> >> 
> >> ad5820: dac@0c {
> >>    compatible = "adi,ad5820";
> >>    reg = <0x0c>;
> >>    
> >>    VANA-supply = <&pm8994_l23>;
> >>    enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>;
> >> };
> > 
> > Well, I'm sure you could have gpio-based regulator powered from
> > pm8994_l23, and outputting to ad5820.
> > 
> > Does ad5820 chip have a gpio input for enable?
> 
> xshutdown pin:
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD5821.pd
> f
> 
> (AD5820,AD5821, and AD5823 are compatibles, or at least that is waht
> the module manufacturer says :)

Is that the pin that msmgpio 26 is connected to on your board ?

I'd argue that the GPIO should be called xshutdown in that case, as DT usually 
uses the pin name, but there's precedent of using well-known names for pins 
with the same functionality. In any case you should update the DT bindings to 
document the new property, and clearly explain that it describes the GPIO 
connected to the xshutdown pin. Please CC the devicetree@vger.kernel.org 
mailing list on the bindings change (they usually like bindings changes to be 
split to a separate patch).

-- 
Regards,

Laurent Pinchart




  reply	other threads:[~2018-09-20 20:04 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20 16:19 [PATCH 1/4] [media] ad5820: Define entity function Ricardo Ribalda Delgado
2018-09-20 16:19 ` [PATCH 2/4] [media] ad5820: Add support for enable pin Ricardo Ribalda Delgado
2018-09-20 18:40   ` Sakari Ailus
2018-09-20 18:47     ` Ricardo Ribalda Delgado
2018-09-20 18:45   ` [PATCH v2 " Ricardo Ribalda Delgado
2018-09-20 18:54     ` Pavel Machek
2018-09-20 19:06       ` Ricardo Ribalda Delgado
2018-09-20 19:08         ` Pavel Machek
2018-09-20 19:12           ` Ricardo Ribalda Delgado
2018-09-20 20:04             ` Laurent Pinchart [this message]
2018-09-20 20:04               ` Laurent Pinchart
2018-09-20 20:14             ` Pavel Machek
2018-09-20 20:21               ` Pavel Machek
2018-09-20 20:32                 ` Ricardo Ribalda Delgado
2018-09-20 16:19 ` [PATCH 3/4] [media] ad5820: DT new optional field enable-gpios Ricardo Ribalda Delgado
2018-09-20 20:21   ` Laurent Pinchart
2018-09-20 20:23     ` Laurent Pinchart
2018-09-20 20:25       ` Ricardo Ribalda Delgado
2018-09-20 20:28         ` Pavel Machek
2018-09-20 16:19 ` [PATCH 4/4] [media] ad5820: Add support for of-autoload Ricardo Ribalda Delgado
2018-09-20 18:31   ` [PATCH v2 " Ricardo Ribalda Delgado
2018-09-20 18:55     ` Pavel Machek
2018-09-20 20:10     ` Laurent Pinchart
2018-09-20 20:18       ` Sakari Ailus
2018-09-20 20:22 ` [PATCH 1/4] [media] ad5820: Define entity function Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1887575.If169ivQRG@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=hans.verkuil@cisco.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=pavel@ucw.cz \
    --cc=ricardo.ribalda@gmail.com \
    --cc=sakari.ailus@iki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox