Re: [PATCH 01/14] Documentation: Add SoundWire summary

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

On Thu, Oct 19, 2017 at 08:33:17AM +0530, Vinod Koul wrote:

> +The SoundWire protocol supports up to eleven Slave interfaces. All the

There's lots of perfectly normal nouns in this document like Slave here
which are randomly capitalized.  Is there some great reason for this?
It makes the document pretty distracting to read.

> +Bus implements API to read standard Master MIPI properties and also provides
> +callback in Master ops for Master driver to implement own functions that

implement it's own functions.

> +provides capabilities information. DT support is not implemented at this
> +time but should be trivial to add since capabilities are enabled with the
> +device_property_ API.

Since we're making this up from whole cloth rather than following an
existing standard let's get a DT binding document together and review
the properties that are getting defined.

> +	/* Check ACPI for Slave devices */
> +        sdw_acpi_find_slaves(bus);

Tab/space issues here.

> +The MIPI specification requires each Slave interface to expose a unique
> +48-bit identifier, stored in 6 read only dev_id registers. This dev_id
> +identifier contains vendor and part information, as well as a field enabling
> +to differentiate between identical components. An additional class field is
> +currently unused. Slave driver is written for the specific 48-bit
> +identifier, Bus enumerates the Slave device based on the 48-bit identifier.

So this says that the instance identifer is part of the device
identifier but the driver should bind to the whole device identifer?
I'd expect the driver to bind to everything except the instance
identifer.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]