Hi Mark, Pratyush

On 4/26/21 6:51 PM, Mark Brown wrote:

On Mon, Apr 26, 2021 at 09:56:12PM +0530, Pratyush Yadav wrote:

quoted

On 26/04/21 04:39PM, patrice.chotard@foss.st.com wrote:

quoted

quoted

+ * spi_mem_poll_status() - Poll memory device status
+ * @mem: SPI memory device
+ * @op: the memory operation to execute
+ * @mask: status bitmask to ckeck
+ * @match: status expected value

quoted

Technically, (status & mask) expected value. Dunno if that is obvious 
enough to not spell out explicitly.

Is it possible there's some situation where you're waiting for some bits
to clear as well?

Yes, we are waiting STATUS_BUSY bit to be cleared, see patch 2 which is making 
usage of this API.

quoted

quoted

+		ret = ctlr->mem_ops->poll_status(mem, op, mask, match, timeout);

I'm not sure I like this name since it makes me think the driver is
going to poll when really it's offloaded to the hardware, but I can't
think of any better ideas either and it *is* what the hardware is going
to be doing so meh.

quoted

I wonder if it is better to let spi-mem core take care of the timeout 
part. On one hand it reduces code duplication on the driver side a 
little bit. Plus it makes sure drivers don't mess anything up with bad 
(or no) handling of the timeout. But on the other hand the interface 
becomes a bit awkward since you'd have to pass a struct completion 
around, and it isn't something particularly hard to get right either. 
What do you think?

We already have the core handling other timeouts.  We don't pass around
completions but rather have an API function that the driver has to call
when the operation completes, a similar pattern might work here.  Part

So, if i correctly understood, you make allusion to what is already done
in SPI core framework with spi_finalize_current_transfer() right ?

of the thing with those APIs which I'm missing here is that this will
just return -EOPNOTSUPP if the driver can't do the delay in hardware, I
think it would be cleaner if this API were similar and the core dealt
with doing the delay/poll on the CPU.  That way the users don't need to
repeat the handling for the offload/non-offload cases.

Sorry, i didn't catch what you mean here. In PATCH 2, that's the case,
if spi_mem_poll_status() is not supported, the core is dealing with 
the delay/poll on the CPU in spinand_wait().

Patrice
Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel