Re: [PATCH 04/15] platform/x86/intel/vsec: Add device links to enforce dependencies

[David E. Box]

Hi Ilpo,

On Tue, 2025-05-20 at 16:51 +0300, Ilpo Järvinen wrote:
> On Wed, 30 Apr 2025, David E. Box wrote:
> 
> > New Intel VSEC features will have dependencies on other features, requiring
> > certain supplier drivers to be probed before their consumers. To enforce
> > this dependency ordering, introduce device links using device_link_add(),
> > ensuring that suppliers are fully registered before consumers are probed.
> > 
> > - Add device link tracking by storing supplier devices and tracking their
> >   state.
> > - Implement intel_vsec_link_devices() to establish links between suppliers
> >   and consumers based on feature dependencies.
> > - Add get_consumer_dependencies() to retrieve supplier-consumer
> >   relationships.
> > - Modify feature registration logic:
> >   * Consumers now check that all required suppliers are registered before
> >     being initialized.
> >   * suppliers_ready() verifies that all required supplier devices are
> >     available.
> > - Prevent potential null consumer name issue in sysfs:
> >   - Use dev_set_name() when creating auxiliary devices to ensure a
> >     unique, non-null consumer name.
> > - Update intel_vsec_pci_probe() to loop up to the number of possible
> >   features or when all devices are registered, whichever comes first.
> > - Introduce VSEC_CAP_UNUSED to prevent sub-features (registered via
> >   exported APIs) from being mistakenly linked.
> > 
> > Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
> > ---

...

> 
> > +		return ret;
> > +	}
> > +
> >  	ret = auxiliary_device_add(auxdev);
> >  	if (ret < 0) {
> >  		auxiliary_device_uninit(auxdev);
> 
> Don't you need to tear down the device link on failure?

Not necessary according to the doc:


 * If DL_FLAG_STATELESS is set in @flags, the caller of this function is
 * expected to release the link returned by it directly with the help of either
 * device_link_del() or device_link_remove().
 *
 * If that flag is not set, however, the caller of this function is handing the
 * management of the link over to the driver core entirely and its return value
 * can only be used to check whether or not the link is present.


I've tested this with repeat modprobe [-r] and kmemleak detect.

I addressed the rest of you comments across the series and will send V2 shortly.

Thanks.

David