All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Janusz Użycki" <j.uzycki@elproma.com.pl>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Richard Genoud <richard.genoud@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	fabio.estevam@freescale.com,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-gpio@vger.kernel.org, linux-serial@vger.kernel.org,
	Fabio Estevam <festevam@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] gpio: mxs: implement get_direction callback
Date: Tue, 18 Nov 2014 10:59:03 +0100	[thread overview]
Message-ID: <546B1867.30308@elproma.com.pl> (raw)
In-Reply-To: <20141117222127.GN27002@pengutronix.de>

Hello Uwe,

W dniu 2014-11-17 o 23:21, Uwe Kleine-König pisze:
> Hello Janusz,
>
> On Mon, Nov 17, 2014 at 08:02:54PM +0100, Janusz Użycki wrote:
>> W dniu 2014-11-17 o 19:42, Uwe Kleine-König pisze:
>>> On Mon, Nov 17, 2014 at 06:07:53PM +0100, Janusz Użycki wrote:
>>>>> It looks there could be one more helper useful.
>>>>> Both atmel_serial.c and mxs-auart.c checks if the line is
>>>>> supported by mctrl_gpio.
>>>>> One time it is eg.:
>>>>> (s->gpio_irq[UART_GPIO_DCD] > 0)
>>>>> another time it is eg.:
>>>>> IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios, UART_GPIO_RTS))
>>>>>
>>>>> The first one is no possible now. The second seems rude.
>>>>> bool mctrl_gpio_is_gpio((struct mctrl_gpios *gpios, enum
>>>>> mctrl_gpio_idx gidx);
>>>>> The name is hard. Moreover the implementation could be very similar
>>>>> to mctrl_gpio_to_gpiod(). Any ideas?
>>>> The differences:
>>>> - faster
>>>> - not exported
>>>> - used mainly in uart's interrupt
>>>> - assumes that struct mctrl_gpios *gpios exists
>>> I wonder why you need it at all?!
>> If the line is not supported by gpio it could be supported by native
>> uart's hardware
>> if possible. There are different configurations. One port has the
>> lines, other
>> which uses the same driver doesn't have.
>> Let's look at disable/enable_ms() in atmet_serial.c.
>> In mxs-auart DMA is not used if CTS or RTS is gpio line (timming).
> For the atmel driver I would expect that it doesn't hurt to set
> ATMEL_US_CTSIC if CTS is realized using a gpio. But maybe I'm too
> optimistic here. So atmel_enable_ms could look as follows:
>
> 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>
> 	/*
> 	 * Interrupt should not be enabled twice
> 	 * ukl: is this check really needed?
> 	 */
> 	if (atmel_port->ms_irq_enabled)
> 		return;
>
> 	atmel_port->ms_irq_enabled = true;
>
> 	mctrl_gpio_enable_ms(...);
> 	UART_PUT_IER(port, ATMEL_US_CTSIC | ATMEL_US_DSRIC | ATMEL_US_RIIC | ATMEL_US_DCDIC);

It is too optimistic I think. We shouldn't assume that internal lines 
are biased if not selected by pinmux.
They should be but it is not ensured in most datasheets. Moreover today 
time to market makes erratas longer
and too often not complete. Sadly but true.

best regards
Janusz
> Best regards
> Uwe
>

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
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: Tue, 18 Nov 2014 10:59:03 +0100	[thread overview]
Message-ID: <546B1867.30308@elproma.com.pl> (raw)
In-Reply-To: <20141117222127.GN27002@pengutronix.de>

Hello Uwe,

W dniu 2014-11-17 o 23:21, Uwe Kleine-K?nig pisze:
> Hello Janusz,
>
> On Mon, Nov 17, 2014 at 08:02:54PM +0100, Janusz U?ycki wrote:
>> W dniu 2014-11-17 o 19:42, Uwe Kleine-K?nig pisze:
>>> On Mon, Nov 17, 2014 at 06:07:53PM +0100, Janusz U?ycki wrote:
>>>>> It looks there could be one more helper useful.
>>>>> Both atmel_serial.c and mxs-auart.c checks if the line is
>>>>> supported by mctrl_gpio.
>>>>> One time it is eg.:
>>>>> (s->gpio_irq[UART_GPIO_DCD] > 0)
>>>>> another time it is eg.:
>>>>> IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios, UART_GPIO_RTS))
>>>>>
>>>>> The first one is no possible now. The second seems rude.
>>>>> bool mctrl_gpio_is_gpio((struct mctrl_gpios *gpios, enum
>>>>> mctrl_gpio_idx gidx);
>>>>> The name is hard. Moreover the implementation could be very similar
>>>>> to mctrl_gpio_to_gpiod(). Any ideas?
>>>> The differences:
>>>> - faster
>>>> - not exported
>>>> - used mainly in uart's interrupt
>>>> - assumes that struct mctrl_gpios *gpios exists
>>> I wonder why you need it at all?!
>> If the line is not supported by gpio it could be supported by native
>> uart's hardware
>> if possible. There are different configurations. One port has the
>> lines, other
>> which uses the same driver doesn't have.
>> Let's look at disable/enable_ms() in atmet_serial.c.
>> In mxs-auart DMA is not used if CTS or RTS is gpio line (timming).
> For the atmel driver I would expect that it doesn't hurt to set
> ATMEL_US_CTSIC if CTS is realized using a gpio. But maybe I'm too
> optimistic here. So atmel_enable_ms could look as follows:
>
> 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>
> 	/*
> 	 * Interrupt should not be enabled twice
> 	 * ukl: is this check really needed?
> 	 */
> 	if (atmel_port->ms_irq_enabled)
> 		return;
>
> 	atmel_port->ms_irq_enabled = true;
>
> 	mctrl_gpio_enable_ms(...);
> 	UART_PUT_IER(port, ATMEL_US_CTSIC | ATMEL_US_DSRIC | ATMEL_US_RIIC | ATMEL_US_DCDIC);

It is too optimistic I think. We shouldn't assume that internal lines 
are biased if not selected by pinmux.
They should be but it is not ensured in most datasheets. Moreover today 
time to market makes erratas longer
and too often not complete. Sadly but true.

best regards
Janusz
> Best regards
> Uwe
>

  reply	other threads:[~2014-11-18  9:58 UTC|newest]

Thread overview: 82+ 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 22:27 ` Janusz Uzycki
2014-11-14 23:26 ` Uwe Kleine-König
2014-11-14 23:26   ` Uwe Kleine-König
2014-11-15 19:29   ` Janusz Użycki
2014-11-15 19:29     ` Janusz Użycki
2014-11-16 21:42     ` Uwe Kleine-König
2014-11-16 21:42       ` Uwe Kleine-König
2014-11-16 21:48       ` Uwe Kleine-König
2014-11-16 21:48         ` Uwe Kleine-König
2014-11-16 23:59       ` Janusz Użycki
2014-11-16 23:59         ` Janusz Użycki
2014-11-17  1:58         ` Janusz Użycki
2014-11-17  1:58           ` Janusz Użycki
2014-11-17  8:28           ` Uwe Kleine-König
2014-11-17  8:28             ` Uwe Kleine-König
2014-11-17  8:38             ` Alexander Shiyan
2014-11-17  8:38               ` Alexander Shiyan
2014-11-17  8:44               ` Uwe Kleine-König
2014-11-17  8:44                 ` Uwe Kleine-König
2014-11-17  8:53                 ` Alexander Shiyan
2014-11-17  8:53                   ` Alexander Shiyan
2014-11-17  9:11             ` Janusz Użycki
2014-11-17  9:11               ` Janusz Użycki
2014-11-17  9:39               ` Uwe Kleine-König
2014-11-17  9:39                 ` Uwe Kleine-König
2014-11-17  9:46               ` Richard Genoud
2014-11-17  9:46                 ` Richard Genoud
2014-11-17  9:59                 ` Uwe Kleine-König
2014-11-17  9:59                   ` Uwe Kleine-König
2014-11-17 10:05                   ` Richard Genoud
2014-11-17 10:05                     ` Richard Genoud
2014-11-17 14:29                     ` Janusz Użycki
2014-11-17 14:29                       ` Janusz Użycki
2014-11-17 16:14                       ` Richard Genoud
2014-11-17 16:14                         ` Richard Genoud
2014-11-17 15:53                     ` Uwe Kleine-König
2014-11-17 15:53                       ` Uwe Kleine-König
2014-11-17 15:58                       ` Janusz Użycki
2014-11-17 15:58                         ` Janusz Użycki
2014-11-17 16:02                         ` Uwe Kleine-König
2014-11-17 16:02                           ` Uwe Kleine-König
2014-11-17 16:04                       ` Richard Genoud
2014-11-17 16:04                         ` Richard Genoud
2014-11-17 17:26                     ` Janusz Użycki
2014-11-17 17:26                       ` Janusz Użycki
2014-11-17 10:05                   ` Alexander Shiyan
2014-11-17 10:05                     ` Alexander Shiyan
2014-11-17 10:09                     ` Russell King - ARM Linux
2014-11-17 10:09                       ` Russell King - ARM Linux
2014-11-17 10:10                     ` Richard Genoud
2014-11-17 10:10                       ` Richard Genoud
2014-11-17 10:17                       ` Russell King - ARM Linux
2014-11-17 10:17                         ` Russell King - ARM Linux
2014-11-17 12:40                 ` Janusz Użycki
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:51                 ` Uwe Kleine-König
2014-11-17  9:57                 ` Richard Genoud
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:00               ` Janusz Użycki
2014-11-17 17:07               ` Janusz Użycki
2014-11-17 17:07                 ` Janusz Użycki
2014-11-17 18:42                 ` Uwe Kleine-König
2014-11-17 18:42                   ` Uwe Kleine-König
2014-11-17 19:02                   ` Janusz Użycki
2014-11-17 19:02                     ` Janusz Użycki
2014-11-17 22:21                     ` Uwe Kleine-König
2014-11-17 22:21                       ` Uwe Kleine-König
2014-11-18  9:59                       ` Janusz Użycki [this message]
2014-11-18  9:59                         ` Janusz Użycki
2014-11-17  9:26         ` Richard Genoud
2014-11-17  9:26           ` Richard Genoud
2014-11-17 14:45           ` Janusz Użycki
2014-11-17 14:45             ` Janusz Użycki
2014-11-17 15:59             ` Uwe Kleine-König
2014-11-17 15:59               ` Uwe Kleine-König
2014-11-17  8:31       ` Richard Genoud
2014-11-17  8:31         ` Richard Genoud
2014-11-17  8:39         ` Uwe Kleine-König
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=546B1867.30308@elproma.com.pl \
    --to=j.uzycki@elproma.com.pl \
    --cc=fabio.estevam@freescale.com \
    --cc=festevam@gmail.com \
    --cc=gnurou@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=richard.genoud@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.