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

Will fix, looked now at wait_event_interruptible_timeout - it cover all cases nicely.

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

Will remove stub helpers, they do more damage than help.

>        Arnd

- - 
Thanks,
Sasha