Re: [PATCH v3 1/4] dt-bindings: soundwire: add slave bindings
From: Rob Herring
Date: Thu Aug 22 2019 - 12:11:24 EST
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