On Tue, Oct 15, 2019 at 7:22 AM Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> wrote: > > > > On 15/10/2019 12:35, Rob Herring wrote: > > On Mon, Oct 14, 2019 at 12:34 PM Srinivas Kandagatla > > <srinivas.kandagatla@xxxxxxxxxx> wrote: > >> > >> Thanks Rob for taking time to review, > >> > >> On 14/10/2019 18:12, Rob Herring wrote: > >>> On Fri, Oct 11, 2019 at 04:44:22PM +0100, Srinivas Kandagatla wrote: > >>>> This patch adds bindings for Qualcomm soundwire controller. > >>>> > >>>> Qualcomm SoundWire Master controller is present in most Qualcomm SoCs > >>>> either integrated as part of WCD audio codecs via slimbus or > >>>> as part of SOC I/O. > >>>> > >>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> > >>>> --- > >>>> .../bindings/soundwire/qcom,sdw.txt | 167 ++++++++++++++++++ > >>>> 1 file changed, 167 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,sdw.txt > >>> > >>> Next time, do a DT schema. > >>> > >> Sure! I can do that in next version! > > > > I meant the next binding you write, not v4. However, ... > > > > [...] > > > >>>> += SoundWire devices > >>>> +Each subnode of the bus represents SoundWire device attached to it. > >>>> +The properties of these nodes are defined by the individual bindings. > >>> > >>> Is there some sort of addressing that needs to be defined? > >>> > >> Thanks, Looks like I missed that here. > >> > >> it should be something like this, > >> > >> #address-cells = <2>; > >> #size-cells = <0>; > >> > >> Will add the in next version. > > > > You need a common soundwire binding for this. You also need to define > > the format of 'reg' and unit addresses. And it needs to be a schema. > > So perhaps this binding too should be. > > We already have a common SoundWire bindings in mainline for this > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/soundwire/soundwire-controller.yaml?h=v5.4-rc3 Indeed... :) > Should this binding just make a reference to it instead of duplicating > this same info here? Yes, that should be sufficient. Rob