linux-kernel - Re: [PATCH v2 2/4] [media] ad5820: Add support for enable pin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1887575.If169ivQRG@avalon>
Date:   Thu, 20 Sep 2018 23:04:21 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
Cc:     Pavel Machek <pavel@....cz>, Sakari Ailus <sakari.ailus@....fi>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media <linux-media@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Hans Verkuil <hans.verkuil@...co.com>
Subject: Re: [PATCH v2 2/4] [media] ad5820: Add support for enable pin

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@....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@....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@...il.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@...r.kernel.org 
mailing list on the bindings change (they usually like bindings changes to be 
split to a separate patch).

-- 
Regards,

Laurent Pinchart