On Thu, Aug 22, 2019 at 7:56 AM Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> wrote: > > > > On 22/08/2019 13:36, Rob Herring wrote: > >>>> +soundwire@c2d0000 { > >>>> + compatible = "qcom,soundwire-v1.5.0" > >>>> + reg = <0x0c2d0000 0x2000>; > >>>> + > >>>> + spkr_left:wsa8810-left{ > >>>> + compatible = "sdw0110217201000"; > >>>> + ... > >>>> + }; > >>>> + > >>>> + spkr_right:wsa8810-right{ > >>>> + compatible = "sdw0120217201000"; > >>> The normal way to distinguish instances is with 'reg'. So I think you > >>> need 'reg' with Instance ID moved there at least. Just guessing, but > >>> perhaps Link ID, too? And for 2 different classes of device is that > >>> enough? > >> In previous bindings (https://lists.gt.net/linux/kernel/3403276 ) we > >> did have instance-id as different property, however Pierre had some good > >> suggestion to make it align with _ADR encoding as per MIPI DisCo spec. > >> > >> Do you still think that we should split the instance id to reg property? > > Assuming you could have more than 1 of the same device on the bus, > > then you need some way to distinguish them and the way that's done for > > DT is unit-address/reg. And compatible strings should be constant for > > each instance. > That is a good point! > Okay that makes more sense keep compatible string constant. > Class ID would be constant for given functionality that the driver will > provide. > > So we will end up with some thing like this: > > soundwire@c2d0000 { > compatible = "qcom,soundwire-v1.5.0" > reg = <0x0c2d0000 0x2000>; > #address-cells = <1>; > #size-cells = <0>; > > spkr_left:skpr@1{ > compatible = "sdw10217201000"; > reg = <0x1> > sdw-link-id = <0>; Not really sure what Link ID is, but maybe it should be part of reg too if it is part of how you address a device. > ... > }; > > spkr_right:spkr@2{ > compatible = "sdw10217201000"; > reg = <0x2> > sdw-link-id = <0>; > }; > }; > > I will spin this in next version! > > Thanks, > srini > > > > > Rob