From: Andrew Lunn <andrew@lunn.ch>
To: Lee Jones <lee.jones@linaro.org>
Cc: linux-kernel@vger.kernel.org, Emeric Dupont <emeric.dupont@zii.aero>
Subject: Re: [PATCH v4] mfd: tqmx86: IO controller with i2c, wachdog and gpio
Date: Fri, 1 Feb 2019 17:49:33 +0100 [thread overview]
Message-ID: <20190201164933.GH30115@lunn.ch> (raw)
In-Reply-To: <20190201150549.GA4973@dell>
> > > For platform data, it should be "*_platform_data" or "*_pdata".
> >
> > It would of been useful if you had said this in the first review,
> > rather than s/data/ddata/, which is rather ambiguous.
>
> How is that ambiguous? I guess it would be confusing if you didn't
> know the syntax, in which case you should have asked.
>
> s/this/that/ simply means, replace 'this' with 'that'.
It is ambiguous if you mean just that one line, one variable, or the
whole file.
> For platform data, it should be "*_platform_data" or "*_pdata".
This is very unambiguous.
> > > > +static uint gpio_irq;
> > > > +module_param(gpio_irq, uint, 0);
> > > > +MODULE_PARM_DESC(gpio_irq, "GPIO IRQ number (7, 9, 12)");
> > >
> > > What is the purpose of providing the IRQ number via a module param?
> > >
> > > These seems like a very bad idea.
> >
> > I explained this in my reply to your review comments for version 2.
> >
> > > Can this driver be built-in?
> >
> > Yes it can. But you can pass module parameters on the command line, so
> > that is not an issue. This is something i actually use.
>
> What is connected to these IRQs?
MODULE_PARM_DESC says:
"GPIO IRQ number (7, 9, 12)"
One of the devices this MFD instantiates is a GPIO controller. Linus
Walleij accepted it a few days ago:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=b868db94a6a704755a33a362cfcf4625abdda115
It can generate interrupts on its GPI's. Unfortunately, the interrupt
the GPIO device uses needs to be configured in the io_ext_int register
which is shared by all devices in the MFD, and it has to unique across
all devices in the MFD. So the module parameter is here, and it then
gets passed to the GPIO driver as a resource in the usual way.
Andrew
prev parent reply other threads:[~2019-02-01 16:49 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-29 20:42 [PATCH v4] mfd: tqmx86: IO controller with i2c, wachdog and gpio Andrew Lunn
2019-02-01 14:15 ` Lee Jones
2019-02-01 14:44 ` Andrew Lunn
2019-02-01 15:05 ` Lee Jones
2019-02-01 16:49 ` Andrew Lunn [this message]
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=20190201164933.GH30115@lunn.ch \
--to=andrew@lunn.ch \
--cc=emeric.dupont@zii.aero \
--cc=lee.jones@linaro.org \
--cc=linux-kernel@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).