linux-kernel - Re: [RFC PATCH 10/13] gpio: bd71828: Initial support for ROHM BD71828 PMIC GPIOs

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: <63a9fca56dbc174bf2c26eb602ba1c0c057488b0.camel@fi.rohmeurope.com>
Date:   Mon, 21 Oct 2019 14:56:11 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>
CC:     "dmurphy@...com" <dmurphy@...com>,
        "linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
        "linux-rtc@...r.kernel.org" <linux-rtc@...r.kernel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
        "mazziesaccount@...il.com" <mazziesaccount@...il.com>,
        "mturquette@...libre.com" <mturquette@...libre.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "jacek.anaszewski@...il.com" <jacek.anaszewski@...il.com>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "a.zummo@...ertech.it" <a.zummo@...ertech.it>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "pavel@....cz" <pavel@....cz>,
        "sboyd@...nel.org" <sboyd@...nel.org>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "lee.jones@...aro.org" <lee.jones@...aro.org>
Subject: Re: [RFC PATCH 10/13] gpio: bd71828: Initial support for ROHM
 BD71828 PMIC GPIOs

Thanks again Bart =)


On Mon, 2019-10-21 at 16:36 +0200, Bartosz Golaszewski wrote:
> pon., 21 paź 2019 o 09:00 Vaittinen, Matti
> <Matti.Vaittinen@...rohmeurope.com> napisał(a):
> > Hello Bartosz,
> > 
> > +
> > > > +       bdgpio->chip.dev = &pdev->dev;
> > > > +       bdgpio->gpio.parent = pdev->dev.parent;
> > > > +       bdgpio->gpio.label = "bd71828-gpio";
> > > > +       bdgpio->gpio.owner = THIS_MODULE;
> > > > +       bdgpio->gpio.get_direction = bd71828_get_direction;
> > > > +       bdgpio->gpio.set_config = bd71828_gpio_set_config;
> > > > +       bdgpio->gpio.can_sleep = true;
> > > > +       bdgpio->gpio.get = bd71828_gpio_get;
> > > > +       bdgpio->gpio.set = bd71828_gpio_set;
> > > 
> > > Not implementing direction_output() and direction_input() here
> > > will
> > > results in warnings from the GPIO framework: for instance you
> > > implement set() but not direction_output(). I'd say: just add
> > > those
> > > callbacks and return an error if they're called for invalid lines
> > > (for
> > > instance: direction_output() being called for line 3).
> > 
> > Ok. I will implement dummy functions.
> > 
> > But out of the curiosity - why the GPIO core emits the warnings if
> > these are not implemented? I think the core should not require "no-
> > operation" functions to be implemented for pins which don't support
> > both of the directions. GPIO core could only emit warning if it
> > needs
> > to set direction to something the HW does not support. That would
> > avoid
> > adding the dummy functions to all of the drivers, right?
> > 
> 
> I looked at the code again and it seems I was wrong. If we don't have
> direction_input() or direction_output() we check the actual direction
> with get_direction() before emitting any warnings and if there's no
> direction_output(), but line is in input mode then all's fine. In
> other words: false alarm, and you can keep it this way.

Thanks for clarifying this - it makes sense :) I wont change this then.

> > > Don't you need a MODULE_ALIAS() here since this is an MFD sub-
> > > module?
> > 
> > I must admit I don't know the details of how module loading is
> > done. I
> > used system where modules are load by scripts. (I guess the module
> > alias could be used to allow automatic module loading [by udev?])
> > 
> > Can you please educate me - If I add module aliases matching the
> > sub-
> > device name given in in MFD cell - should the sub module loading be
> > automatic when MFD driver gets probed? For some reason I didn't get
> > that working on my test bed. Or maybe I misunderstood something.
> > 
> 
> If the gpio module is a sub-node on the device tree than you may need
> to use a sub-compatible to get the module loaded by udev.

Just found out that the last update broke my test bed I2C completely.
I'll experiment with the MODULE_ALIAS when I get my board running...

I don't want to add own DT node for gpio (and all other sub devices).
That shouldn't be needed. It really should be enough to kick the MFD
part from DT using the compatible - subdevices should be load by MFD
without having own DT compatibles for them. But I'll see how this works
out - thanks!

Br,
	Matti Vaittinen