On 26/04/21 05:51PM, Mark Brown wrote: > On Mon, Apr 26, 2021 at 09:56:12PM +0530, Pratyush Yadav wrote: > > On 26/04/21 04:39PM, patrice.chotard@xxxxxxxxxxx wrote: > > > > + * 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 > > > 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. In fact, that is the more common situation. Both SPI NOR (spi_nor_sr_ready()) and SPI NAND (spinand_wait()) need to wait for the "busy" bit to be cleared. AFAICT this API is supposed to check for (status & mask) == (match & mask) so it should be able to handle both polarities for the bits being polled. > > > > + 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. > > > 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 > 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. Makes sense to me. -- Regards, Pratyush Yadav Texas Instruments Inc.