From: j.uzycki@elproma•com.pl (Janusz Użycki)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] gpio: mxs: implement get_direction callback
Date: Mon, 17 Nov 2014 10:11:25 +0100 [thread overview]
Message-ID: <5469BBBD.30003@elproma.com.pl> (raw)
In-Reply-To: <20141117082848.GZ27002@pengutronix.de>
W dniu 2014-11-17 o 09:28, Uwe Kleine-K?nig pisze:
> Hello Janusz,
>
> On Mon, Nov 17, 2014 at 02:58:44AM +0100, Janusz U?ycki wrote:
>> W dniu 2014-11-17 o 00:59, Janusz U?ycki pisze:
>>> W dniu 2014-11-16 o 22:42, Uwe Kleine-K?nig pisze:
>>> Thanks Uwe. I fully agree with you.
>>> a) was just a starter to your suggestion. My options were too
>>> conservative - I just
>>> wanted to avoid tests on hardware I don't have.
> That's something you have to live with and that's why there is a merge
> window.
>
>>> I don't understand why gpiod_get_direction() always requires the callback
>>> and b) would be broken (I'm not so familiar with gpiolib) but I
>>> don't need it now.
>>>
>>> So, it looks we can drop the gpio-mxs patch, yes?
> That patch is not wrong, just its motivation. IMHO the only valid
> usecase for .get_direction is debugging.
OK, I will submit v2.
>
>>> And, I or Richard should submit a patch for
>>> mctrl_gpio/atmel_serial/mxs-auart
>>> to introduce the irq helper, yes?
>>>
>>> You wrote passing uart_port is enough. Argument "name" for
>>> request_irq() can be
>>> recovered from dev_name(dev) or dev_driver_string(dev) where dev
>>> = port_uart->dev.
>>> But irqhandler and mctrl_gpios must be passed to
> You don't need irqhandler. struct mctrl_gpios is needed of course.
request_irq() needs a irqhandler. Do you thing about a mctrl_ handler
for gpios?
>
>>> mctrl_gpio_request_irqs() helper.
>>> The gpio_irq table could be hidden and moved into struct
>>> mctrl_gpios. Then
>>> a second helper function is required: mctrl_gpio_free_irqs().
> yes.
>
>> After some coding...
>> gpio_irq cannot be hidden - it is used by disable/enable_ms() and
>> not only :/
> mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
This makes unable to combine gpio's and chip's lines but it could be
advantage
to separate them.
>
>>> gpio_irq table initialized in mctrl_gpio_request_irqs().
>> or it could be nicely done in mctrl_gpio_init() but the problem is
>> next argument
>> for the function :/
>> eg.:
>> struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int
>> idx, int *irqs)
> What is idx about? I see it already in the mctrl_gpio API, but there is
> no documentation about how it's used. Is it always 0?
dt index
> There is no need to pass an output parameter for irqs. Just save them in
> struct mctrl_gpios.
>
> I'd go and change all struct device * parameters of the mctrl_gpio API
> to struct uart_port for consistency or add struct uart_port to struct
> mctrl_gpios.
>
>>> So finally the prototypes would be:
>>> int mctrl_gpio_request_irqs(struct mctrl_gpios*, struct
>>> uart_port*, irqhandler_t);
>>> void mctrl_gpio_free_irqs(struct mctrl_gpios*);
> I think:
>
> struct mctrl_gpios {
> struct uart_port *port;
> struct {
> gpio_desc *gpio;
> unsigned int irq;
> } mctrl_line[UART_GPIO_MAX];
> };
Looks good. Richard, do you agree?
> struct mctrl_gpios *mctrl_gpio_init(struct uart_port *port, unsigned int idx_if_needed);
> int mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
> int mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
> void mctrl_gpio_free(struct mctrl_gpios *gpios);
>
> I think mctrl_gpio_init should request the needed irqs, but not enable
> them.
Yes. I tried to assign irq value in mctrl_gpio_init() only.
There was another issue if CONFIG_GPIOLIB is not defined but it looks
mctrl_ disable/enable_ms()
and mctrl_ irq handler solve the problem.
> Not sure there is a corresponding request_irq variant for that.
What would you propose?
> Another open issue is how mctrl_gpio_init should find out about which
> gpios to use if there is no device tree. This doesn't necessarily needs
> to be solved now, but maybe rename mctrl_gpio_init to
> mctrl_gpio_init_dt?
Right
best regards
Janusz
>
> Best regards
> Uwe
>
next prev parent reply other threads:[~2014-11-17 9:11 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-14 22:27 [PATCH] gpio: mxs: implement get_direction callback Janusz Uzycki
2014-11-14 23:26 ` Uwe Kleine-König
2014-11-15 19:29 ` Janusz Użycki
2014-11-16 21:42 ` Uwe Kleine-König
2014-11-16 21:48 ` Uwe Kleine-König
2014-11-16 23:59 ` Janusz Użycki
2014-11-17 1:58 ` Janusz Użycki
2014-11-17 8:28 ` Uwe Kleine-König
2014-11-17 8:38 ` Alexander Shiyan
2014-11-17 8:44 ` Uwe Kleine-König
2014-11-17 8:53 ` Alexander Shiyan
2014-11-17 9:11 ` Janusz Użycki [this message]
2014-11-17 9:39 ` Uwe Kleine-König
2014-11-17 9:46 ` Richard Genoud
2014-11-17 9:59 ` Uwe Kleine-König
2014-11-17 10:05 ` Richard Genoud
2014-11-17 14:29 ` Janusz Użycki
2014-11-17 16:14 ` Richard Genoud
2014-11-17 15:53 ` Uwe Kleine-König
2014-11-17 15:58 ` Janusz Użycki
2014-11-17 16:02 ` Uwe Kleine-König
2014-11-17 16:04 ` Richard Genoud
2014-11-17 17:26 ` Janusz Użycki
2014-11-17 10:05 ` Alexander Shiyan
2014-11-17 10:09 ` Russell King - ARM Linux
2014-11-17 10:10 ` Richard Genoud
2014-11-17 10:17 ` Russell King - ARM Linux
2014-11-17 12:40 ` Janusz Użycki
2014-11-17 9:51 ` request an irq without enabling? [Was: Re: [PATCH] gpio: mxs: implement get_direction callback] Uwe Kleine-König
2014-11-17 9:57 ` Richard Genoud
2014-11-17 17:00 ` [PATCH] gpio: mxs: implement get_direction callback Janusz Użycki
2014-11-17 17:07 ` Janusz Użycki
2014-11-17 18:42 ` Uwe Kleine-König
2014-11-17 19:02 ` Janusz Użycki
2014-11-17 22:21 ` Uwe Kleine-König
2014-11-18 9:59 ` Janusz Użycki
2014-11-17 9:26 ` Richard Genoud
2014-11-17 14:45 ` Janusz Użycki
2014-11-17 15:59 ` Uwe Kleine-König
2014-11-17 8:31 ` Richard Genoud
2014-11-17 8:39 ` Uwe Kleine-König
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=5469BBBD.30003@elproma.com.pl \
--to=j.uzycki@elproma$(echo .)com.pl \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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