From: Boris Brezillon <boris.brezillon@collabora.com>
To: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Mark Brown <broonie@kernel.org>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Vignesh Raghavendra <vigneshr@ti.com>,
<linux-mtd@lists.infradead.org>,
Alexandre Torgue <alexandre.torgue@foss.st.com>,
<linux-spi@vger.kernel.org>,
<linux-stm32@st-md-mailman.stormreply.com>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <christophe.kerello@foss.st.com>
Subject: Re: [PATCH v2 1/3] spi: spi-mem: add automatic poll status functions
Date: Mon, 10 May 2021 11:22:49 +0200 [thread overview]
Message-ID: <20210510112249.5613978e@collabora.com> (raw)
In-Reply-To: <542000b4-1a65-5090-72f9-441c75ee1098@foss.st.com>
On Mon, 10 May 2021 10:46:48 +0200
Patrice CHOTARD <patrice.chotard@foss.st.com> wrote:
> >
> >> +
> >> + if (ctlr->mem_ops && ctlr->mem_ops->poll_status) {
> >> + ret = spi_mem_access_start(mem);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + reinit_completion(&ctlr->xfer_completion);
> >> +
> >> + ret = ctlr->mem_ops->poll_status(mem, op, mask, match,
> >> + timeout_ms);
> >> +
> >> + ms = wait_for_completion_timeout(&ctlr->xfer_completion,
> >> + msecs_to_jiffies(timeout_ms));
> >
> > Why do you need to wait here? I'd expect the poll_status to take care
> > of this wait.
>
> It was a request from Mark Brown [1]. The idea is to implement
> similar mechanism already used in SPI framework.
Well, you have to choose, either you pass a timeout to ->poll_status()
and let the driver wait for the status change (and return -ETIMEDOUT if
it didn't happen in time), or you do it here and the driver only has to
signal the core completion object. I think it's preferable to let the
driver handle the timeout though, because you don't know how the
status check will be implemented, and it's not like the
reinit_completion()+wait_for_completion_timeout() done here would
greatly simplify the drivers wait logic anyway.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Mark Brown <broonie@kernel.org>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Vignesh Raghavendra <vigneshr@ti.com>,
<linux-mtd@lists.infradead.org>,
Alexandre Torgue <alexandre.torgue@foss.st.com>,
<linux-spi@vger.kernel.org>,
<linux-stm32@st-md-mailman.stormreply.com>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <christophe.kerello@foss.st.com>
Subject: Re: [PATCH v2 1/3] spi: spi-mem: add automatic poll status functions
Date: Mon, 10 May 2021 11:22:49 +0200 [thread overview]
Message-ID: <20210510112249.5613978e@collabora.com> (raw)
In-Reply-To: <542000b4-1a65-5090-72f9-441c75ee1098@foss.st.com>
On Mon, 10 May 2021 10:46:48 +0200
Patrice CHOTARD <patrice.chotard@foss.st.com> wrote:
> >
> >> +
> >> + if (ctlr->mem_ops && ctlr->mem_ops->poll_status) {
> >> + ret = spi_mem_access_start(mem);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + reinit_completion(&ctlr->xfer_completion);
> >> +
> >> + ret = ctlr->mem_ops->poll_status(mem, op, mask, match,
> >> + timeout_ms);
> >> +
> >> + ms = wait_for_completion_timeout(&ctlr->xfer_completion,
> >> + msecs_to_jiffies(timeout_ms));
> >
> > Why do you need to wait here? I'd expect the poll_status to take care
> > of this wait.
>
> It was a request from Mark Brown [1]. The idea is to implement
> similar mechanism already used in SPI framework.
Well, you have to choose, either you pass a timeout to ->poll_status()
and let the driver wait for the status change (and return -ETIMEDOUT if
it didn't happen in time), or you do it here and the driver only has to
signal the core completion object. I think it's preferable to let the
driver handle the timeout though, because you don't know how the
status check will be implemented, and it's not like the
reinit_completion()+wait_for_completion_timeout() done here would
greatly simplify the drivers wait logic anyway.
WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Mark Brown <broonie@kernel.org>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Vignesh Raghavendra <vigneshr@ti.com>,
<linux-mtd@lists.infradead.org>,
Alexandre Torgue <alexandre.torgue@foss.st.com>,
<linux-spi@vger.kernel.org>,
<linux-stm32@st-md-mailman.stormreply.com>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <christophe.kerello@foss.st.com>
Subject: Re: [PATCH v2 1/3] spi: spi-mem: add automatic poll status functions
Date: Mon, 10 May 2021 11:22:49 +0200 [thread overview]
Message-ID: <20210510112249.5613978e@collabora.com> (raw)
In-Reply-To: <542000b4-1a65-5090-72f9-441c75ee1098@foss.st.com>
On Mon, 10 May 2021 10:46:48 +0200
Patrice CHOTARD <patrice.chotard@foss.st.com> wrote:
> >
> >> +
> >> + if (ctlr->mem_ops && ctlr->mem_ops->poll_status) {
> >> + ret = spi_mem_access_start(mem);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + reinit_completion(&ctlr->xfer_completion);
> >> +
> >> + ret = ctlr->mem_ops->poll_status(mem, op, mask, match,
> >> + timeout_ms);
> >> +
> >> + ms = wait_for_completion_timeout(&ctlr->xfer_completion,
> >> + msecs_to_jiffies(timeout_ms));
> >
> > Why do you need to wait here? I'd expect the poll_status to take care
> > of this wait.
>
> It was a request from Mark Brown [1]. The idea is to implement
> similar mechanism already used in SPI framework.
Well, you have to choose, either you pass a timeout to ->poll_status()
and let the driver wait for the status change (and return -ETIMEDOUT if
it didn't happen in time), or you do it here and the driver only has to
signal the core completion object. I think it's preferable to let the
driver handle the timeout though, because you don't know how the
status check will be implemented, and it's not like the
reinit_completion()+wait_for_completion_timeout() done here would
greatly simplify the drivers wait logic anyway.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-10 9:41 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-07 13:17 [PATCH v2 0/3] MTD: spinand: Add spi_mem_poll_status() support patrice.chotard
2021-05-07 13:17 ` patrice.chotard
2021-05-07 13:17 ` patrice.chotard
2021-05-07 13:17 ` [PATCH v2 1/3] spi: spi-mem: add automatic poll status functions patrice.chotard
2021-05-07 13:17 ` patrice.chotard
2021-05-07 13:17 ` patrice.chotard
2021-05-08 7:55 ` Boris Brezillon
2021-05-08 7:55 ` Boris Brezillon
2021-05-08 7:55 ` Boris Brezillon
2021-05-10 8:46 ` Patrice CHOTARD
2021-05-10 8:46 ` Patrice CHOTARD
2021-05-10 8:46 ` Patrice CHOTARD
2021-05-10 9:22 ` Boris Brezillon [this message]
2021-05-10 9:22 ` Boris Brezillon
2021-05-10 9:22 ` Boris Brezillon
2021-05-17 7:29 ` Patrice CHOTARD
2021-05-17 7:29 ` Patrice CHOTARD
2021-05-17 7:29 ` Patrice CHOTARD
2021-05-17 7:41 ` Boris Brezillon
2021-05-17 7:41 ` Boris Brezillon
2021-05-17 7:41 ` Boris Brezillon
2021-05-17 9:24 ` Patrice CHOTARD
2021-05-17 9:24 ` Patrice CHOTARD
2021-05-17 9:24 ` Patrice CHOTARD
2021-05-17 11:25 ` Boris Brezillon
2021-05-17 11:25 ` Boris Brezillon
2021-05-17 11:25 ` Boris Brezillon
2021-05-17 11:59 ` Patrice CHOTARD
2021-05-17 11:59 ` Patrice CHOTARD
2021-05-17 11:59 ` Patrice CHOTARD
2021-05-17 12:24 ` Boris Brezillon
2021-05-17 12:24 ` Boris Brezillon
2021-05-17 12:24 ` Boris Brezillon
2021-05-17 12:04 ` Patrice CHOTARD
2021-05-17 12:04 ` Patrice CHOTARD
2021-05-17 12:04 ` Patrice CHOTARD
2021-05-07 13:17 ` [PATCH v2 2/3] mtd: spinand: use the spi-mem poll status APIs patrice.chotard
2021-05-07 13:17 ` patrice.chotard
2021-05-07 13:17 ` patrice.chotard
2021-05-07 13:17 ` [PATCH v2 3/3] spi: stm32-qspi: add automatic poll status feature patrice.chotard
2021-05-07 13:17 ` patrice.chotard
2021-05-07 13:17 ` patrice.chotard
2021-05-07 19:29 ` kernel test robot
2021-05-07 19:29 ` kernel test robot
2021-05-07 19:29 ` kernel test robot
2021-05-07 19:29 ` kernel test robot
2021-05-07 22:16 ` kernel test robot
2021-05-07 22:16 ` kernel test robot
2021-05-07 22:16 ` kernel test robot
2021-05-07 22:16 ` kernel test robot
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=20210510112249.5613978e@collabora.com \
--to=boris.brezillon@collabora.com \
--cc=alexandre.torgue@foss.st.com \
--cc=broonie@kernel.org \
--cc=christophe.kerello@foss.st.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=linux-spi@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=miquel.raynal@bootlin.com \
--cc=patrice.chotard@foss.st.com \
--cc=vigneshr@ti.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 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.