| 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
| ||
|
Message-ID: <20180605195854.GA16394@rob-hp-laptop>
Date: Tue, 5 Jun 2018 13:58:54 -0600
From: Rob Herring <robh@...nel.org>
To: Levin Du <djw@...hip.com.cn>
Cc: "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
Wayne Chou <zxf@...hip.com.cn>,
Heiko Stuebner <heiko@...ech.de>, devicetree@...r.kernel.org,
Linus Walleij <linus.walleij@...aro.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
Mark Rutland <mark.rutland@....com>,
"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 2/5] gpio: syscon: rockchip: add GPIO_MUTE support for
rk3328
On Sat, Jun 02, 2018 at 04:40:09PM +0800, Levin Du wrote:
>
> Rob Herring <robh+dt@...nel.org> writes:
>
> > On Thu, May 31, 2018 at 9:05 PM, Levin <djw@...hip.com.cn> wrote:
> > > Hi Rob,
> > >
> > >
> > > On 2018-05-31 10:45 PM, Rob Herring wrote:
> > > >
> > > > On Wed, May 30, 2018 at 10:27 PM, <djw@...hip.com.cn> wrote:
> > > > >
> > > > > From: Levin Du <djw@...hip.com.cn>
> > > > >
> > > > > In Rockchip RK3328, the output only GPIO_MUTE pin,
> > > > > originally for codec
> > > > > mute control, can also be used for general purpose. It is
> > > > > manipulated by
> > > > > the GRF_SOC_CON10 register.
> > > > >
> > > > > Signed-off-by: Levin Du <djw@...hip.com.cn>
> > > > >
> > > > > ---
> > > > >
> > > > > Changes in v3:
> > > > > - Change from general gpio-syscon to specific
> > > > > rk3328-gpio-mute
> > > > >
> > > > > Changes in v2:
> > > > > - Rename gpio_syscon10 to gpio_mute in doc
> > > > >
> > > > > Changes in v1:
> > > > > - Refactured for general gpio-syscon usage for Rockchip SoCs.
> > > > > - Add doc rockchip,gpio-syscon.txt
> > > > >
> > > > > .../bindings/gpio/rockchip,rk3328-gpio-mute.txt | 28
> > > > > +++++++++++++++++++
> > > > > drivers/gpio/gpio-syscon.c | 31
> > > > > ++++++++++++++++++++++
> > > > > 2 files changed, 59 insertions(+)
> > > > > create mode 100644
> > > > > Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
> > > > > b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
> > > > > new file mode 100644
> > > > > index 0000000..10bc632
> > > > > --- /dev/null
> > > > > +++
> > > > > b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
> > > > > @@ -0,0 +1,28 @@
> > > > > +Rockchip RK3328 GPIO controller dedicated for the GPIO_MUTE
> > > > > pin.
> > > > > +
> > > > > +In Rockchip RK3328, the output only GPIO_MUTE pin,
> > > > > originally for codec
> > > > > mute
> > > > > +control, can also be used for general purpose. It is
> > > > > manipulated by the
> > > > > +GRF_SOC_CON10 register.
> > > > > +
> > > > > +Required properties:
> > > > > +- compatible: Should contain "rockchip,rk3328-gpio-mute".
> > > > > +- gpio-controller: Marks the device node as a gpio
> > > > > controller.
> > > > > +- #gpio-cells: Should be 2. The first cell is the pin
> > > > > number and
> > > > > + the second cell is used to specify the gpio polarity:
> > > > > + 0 = Active high,
> > > > > + 1 = Active low.
> > > > > +
> > > > > +Example:
> > > > > +
> > > > > + grf: syscon@...00000 {
> > > > > + compatible = "rockchip,rk3328-grf", "syscon",
> > > > > "simple-mfd";
> > > > > +
> > > > > + gpio_mute: gpio-mute {
> > > >
> > > > Node names should be generic:
> > > >
> > > > gpio {
> > > >
> > > > This also means you can't add another GPIO node in the future
> > > > and
> > > > you'll have to live with "rockchip,rk3328-gpio-mute" covering
> > > > more
> > > > than 1 GPIO if you do need to add more GPIOs.
> > >
> > >
> > > As the first line describes, this GPIO controller is dedicated for
> > > the
> > > GPIO_MUTE pin.
> > > There's only one GPIO pin in the GRF_SOC_CON10 register. Therefore
> > > the
> > > gpio_mute
> > > name is proper IMHO.
> >
> > It's how many GPIOs in the GRF, not this register. What I'm saying is
> > when you come along later to add another GPIO in the GRF, you had
> > better just add it to this same node. I'm not going to accept another
> > GPIO controller node within the GRF. You have the cells to support
> > more than 1, so it would only be a driver change. The compatible
> > string would then not be ideally named at that point. But compatible
> > strings are just unique identifiers, so it doesn't really matter what
> > the string is.
> >
>
> I'll try my best to introduce the situation here. The GRF, GPIO0~GPIO3
> are register blocks in the RK3328 Soc. The GPIO0~GPIO3 contain registers
> for GPIO operations like reading/writing data, setting direction,
> interruption etc, which corresponds to the GPIO banks (gpio0~gpio3)
> defined in rk3328.dtsi:
I'm only talking about GRF functions, not "regular" GPIOs.
> pinctrl: pinctrl {
> compatible = "rockchip,rk3328-pinctrl";
> rockchip,grf = <&grf>;
> #address-cells = <2>;
> #size-cells = <2>;
> ranges;
>
> gpio0: gpio0@...10000 {
> compatible = "rockchip,gpio-bank";
> reg = <0x0 0xff210000 0x0 0x100>;
> interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&cru PCLK_GPIO0>;
>
> gpio-controller;
> #gpio-cells = <2>;
>
> interrupt-controller;
> #interrupt-cells = <2>;
> };
>
> gpio1: gpio1@...20000 {
> //...
> };
>
> gpio2: gpio2@...30000 {
> //...
> };
>
> gpio3: gpio3@...40000 {
> //...
> };
> }
>
> However, these general GPIO pins has multiplexed functions and their
> pull up/down and driving strength can also be configured. These settings
> are manipulated by the GRF registers in pinctrl driver. Quoted from the
> TRM, the GRF has the following function:
>
> - IOMUX control
> - Control the state of GPIO in power-down mode
> - GPIO PAD pull down and pull up control
> - Used for common system control
> - Used to record the system state
>
> Therefore the functions of the GRF are messy and scattered in different
> nodes. The so-called GPIO_MUTE does not belong to GPIO0~GPIO3. It is
> manipulated by the GRF_SOC_CON10 register in the GRF block.
>
> > I'm being told both "this is the only GPIO" and "the GRF has too many
> > different functions for us to tell you what they all are". So which is
> > it?
> >
> > Rob
>
> They are both true, but lack of context. See the above description.
What I meant was "only GPIO in GRF registers"...
Rob
Powered by blists - more mailing lists