From: "Jan Kundrát" <jan.kundrat@cesnet.cz>
To: "Mylène Josserand" <mylene.josserand@bootlin.com>
Cc: <gregkh@linuxfoundation.org>, <robh+dt@kernel.org>,
<mark.rutland@arm.com>, <linux-serial@vger.kernel.org>,
<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH v1] tty: serial: max310x: Add optional reset gpio
Date: Tue, 02 Jul 2019 16:44:20 +0200 [thread overview]
Message-ID: <d9ac1295-e780-409d-b7de-b4c2db586e58@cesnet.cz> (raw)
In-Reply-To: <20190614141112.29962-1-mylene.josserand@bootlin.com>
On pátek 14. června 2019 16:11:12 CEST, Mylène Josserand wrote:
> --- a/Documentation/devicetree/bindings/serial/maxim,max310x.txt
> +++ b/Documentation/devicetree/bindings/serial/maxim,max310x.txt
> @@ -15,6 +15,7 @@ Required properties:
> "osc" if an external clock source is used.
>
> Optional properties:
> +- reset-gpios: Gpio to use for reset.
"GPIO", not "Gpio", for consistency.
> if (spi->dev.of_node) {
> + struct gpio_desc *reset_gpio;
> const struct of_device_id *of_id =
> of_match_device(max310x_dt_ids, &spi->dev);
> if (!of_id)
> return -ENODEV;
>
> devtype = (struct max310x_devtype *)of_id->data;
> + reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(reset_gpio))
> + return PTR_ERR(reset_gpio);
> + gpiod_set_value_cansleep(reset_gpio, 0);
> } else {
The RST signal is active-low on the chip, but the code initializes the
output to GPIOD_OUT_HIGH. Are you perhaps relying on a DT binding setting
an ACTIVE_LOW flag on the reset GPIO lane? This should be documented.
Assuming that this polarity inversion works, the code first asserts the
reset, then it performs no explicit waiting, and then it clears the RST
signal. I checked MAX14830's datasheet, and there's no minimal reset
duration, so perhaps this is safe, but it looks a bit odd to me.
With kind regards,
Jan
prev parent reply other threads:[~2019-07-02 14:52 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-14 14:11 [PATCH v1] tty: serial: max310x: Add optional reset gpio Mylène Josserand
2019-07-02 14:44 ` Jan Kundrát [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=d9ac1295-e780-409d-b7de-b4c2db586e58@cesnet.cz \
--to=jan.kundrat@cesnet.cz \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mylene.josserand@bootlin.com \
--cc=robh+dt@kernel.org \
--cc=thomas.petazzoni@bootlin.com \
/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).