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

[Arnd Bergmann]

On Thu, Oct 15, 2026, at 14:47, Usyskin, Alexander wrote:
>> 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;
>> > +		}
>> 
>> 
>> 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.

But if it times out, it won't be in that state unless you hit
a tiny race, for consistency I would always check the return
code, not check the condition again.

>> > +#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.

Yes

> Caller anyway should wait for device with issei_device_register_interface() and
> it will receive devices as soon as module will be loaded.

If that configuration is allowed, then calling any of the exported
functions causes a link error. Your only options really are:

a) prevent the broken config using Kconfig
   'depends on INTEL_SSEI || !INTEL_SSEI' for every caller to force
   the caller to e a module instead of built-in driver.

b) remove the stub helpers and use a simpler 'depends on INTEL_SSEI'

       Arnd