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 13:40:40 +0100 [thread overview]
Message-ID: <5469ECC8.3080301@elproma.com.pl> (raw)
In-Reply-To: <CACQ1gAha1MuroyB35zN_BuwRNGS_J3Xh1htiHJda_2_19qh31g@mail.gmail.com>
W dniu 2014-11-17 o 10:46, Richard Genoud pisze:
> 2014-11-17 10:11 GMT+01:00 Janusz U?ycki <j.uzycki@elproma•com.pl>:
>> W dniu 2014-11-17 o 09:28, Uwe Kleine-K?nig pisze:
>>
>>>>> 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.
>>
>> [...]
>>> 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.
>>
>> [...]
>>> 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;
> I think it's just "int irq;" there
>>> } mctrl_line[UART_GPIO_MAX];
>>> };
>>
>> Looks good. Richard, do you agree?
> yes, seems ok too me !
What do you prefer?
struct mctrl_gpios {
struct uart_port *port;
struct {
struct gpio_desc *gpio;
unsigned int irq;
} mline[UART_GPIO_MAX];
};
or
struct mctrl_gpios {
struct uart_port *port;
struct gpio_desc *gpio[UART_GPIO_MAX];
unsigned int irq[UART_GPIO_MAX];
};
The second one allows to save a lot of current code and make the code
shorter.
best regards
Janusz
>
>>> 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?
> In atmel_request_gpio_irq(), the function irq_set_status_flags(irq,
> IRQ_NOAUTOEN); is used before request_irq to prevent the irq from
> being enabled when requested.
>
>>> 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 12:40 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
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 [this message]
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=5469ECC8.3080301@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