RE: [PATCH char-misc-next 4/4] issei: add driver to driver interface

[Usyskin, Alexander]

> Subject: Re: [PATCH char-misc-next 4/4] issei: add driver to driver interface
> 
> On Thu, Oct 15, 2026, at 07:28, Alexander Usyskin wrote:
> 
> > +	if (idev->rst_state != ISSEI_RST_STATE_DONE) {
> > +		wait_event_interruptible_timeout(idev->wait_rst_state,
> > +						 idev->rst_state ==
> ISSEI_RST_STATE_DONE,
> > +
> msecs_to_jiffies(ISSEI_DEVICE_CONNECT_TO_MS));
> > +		if (idev->rst_state != ISSEI_RST_STATE_DONE) {
> > +			dev_dbg(&idev->dev, "Device is in transition\n");
> > +			return -ENODEV;
> > +		}
> 
> 
> > +		wret = wait_event_interruptible_timeout(cl->write_wait,
> > +
> 	issei_cl_check_write(cl) != 1,
> > +
> 	msecs_to_jiffies(timeout));
> > +		if (wret <= 0) {
> > +			issei_cl_clean_all_wbuf(cl);
> > +			if (wret == 0) /* timed out */
> > +				return -ETIMEDOUT;
> > +			if (signal_pending(current))
> > +				return -EINTR;
> > +			return -ERESTARTSYS;
> 
> The wait logic is inconsistent here, the first one ignores the timeout
> and pending signal, the second one flips -ERESTARTSYS to -EINTR and
> everything else to -ERESTARTSYS, which is also questionable.
> 

First wait ignores timeout as it only interesting in ISSEI_RST_STATE_DONE.
If after timeout the state is ISSEI_RST_STATE_DONE - we can continue in flow.
About signal - you are right - better to add check and specified return code.

Second wait was converted from wait_event_interruptible and I've missed
difference in return values - will fix this.

> > +#if IS_ENABLED(CONFIG_INTEL_SSEI)
> > +
> > +void *issei_device_open(int (*match)(struct device *, const void *),
> > const void *data);
> > +void issei_device_release(void *ctx);
> ...
> > +#else
> > +
> > +void *issei_device_open(int (*match)(struct device *, const void *),
> > const void *data)
> > +{
> > +	return ERR_PTR(-EOPNOTSUPP);
> > +}
> 
> What is the intended use case for !CONFIG_INTEL_SSEI in the caller,
> and what do you expect to happen when CONFIG_INTEL_SSEI=m but the
> caller is configured as built-in?
> 

For CONFIG_INTEL_SSEI=m #if IS_ENABLED(CONFIG_INTEL_SSEI) will be true, I hope.
Caller anyway should wait for device with issei_device_register_interface() and
it will receive devices as soon as module will be loaded.

>       Arnd

- - 
Thanks,
Sasha