On Wed, Sep 30, 2020 at 03:50:46PM -0700, Dave Ertman wrote:

>  Documentation/driver-api/ancillary_bus.rst | 230 +++++++++++++++++++++
>  Documentation/driver-api/index.rst         |   1 +

It would probably be useful to have the documentation in a separate
patch, it's a huge proportion of the patch and would make it much more
approachable.

> +are controlled by DT/ACPI. The same argument applies for not using MFD in this
> +scenario as MFD relies on individual function devices being physical devices
> +that are DT enumerated.

See my commments on the cover letter about MFD, this is just not true.

> +An example for this kind of requirement is the audio subsystem where a single
> +IP is handling multiple entities such as HDMI, Soundwire, local devices such as
> +mics/speakers etc. The split for the core's functionality can be arbitrary or

This is not a requirement of the audio subsystem, this is to do with how
the Intel audio hardware has been implemented on their modern SoCs.

> +int ancillary_device_initialize(struct ancillary_device *ancildev)
> +{

> +int __ancillary_device_add(struct ancillary_device *ancildev, const char *modname)
> +{

It can be useful to have this split but there's also going to be plenty
of cases where people just need to register a device based on the struct
ancilliary_device straight away so it would be good to at least have a
standard ancilliary_device_new() (or whatever) that does both steps in
one.  As Greg said in his review this split model is a bit more fiddly
to use and frequently leads to error handling problems in drivers.