On Fri, Feb 21, 2014 at 09:33:52AM +0000, Mark Rutland wrote: > On Fri, Feb 21, 2014 at 06:43:05AM +0000, Andy Gross wrote: [snip] > > + bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk"); > > The binding document should describe the "bam_clk" string in the > clock-names description. > OK. > > + ret = of_property_read_u32(pdev->dev.of_node, "qcom,ee", &bdev->ee); > > + if (ret) { > > + dev_err(bdev->dev, "EE unspecified\n"); > > It would be nice to expand EE to what it stands for, at least in the > error but possibly the binding too. I can expand it to execution environment. This is a trustzone thing and is required from a security perspective. > > +static const struct of_device_id bam_of_match[] = { > > + { .compatible = "qcom,bam-v1.4.0", }, > > + { .compatible = "qcom,bam-v1.4.1", }, > > What are the differences between the two? > > Is v1.4.1 an extension of v1.4.0? On second look, the 1.4.1 has hardware fixes. So the software API is the same. I thought there were some configuration changes, but that isn't the case. > Could you have the following in DTs: > > compatible = "qcom,bam-v1.4.1", "qcom,bam-v1.4.0"; > > Then you'd only need the v1.4.0 string in the driver for now. Good point. That's easy to change. -- sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html