On 08/13, Rajendra Nayak wrote: > >On 11/08/15 23:49, Stephen Boyd wrote: > >>On 07/08, Rajendra Nayak wrote: > >>>diff --git a/drivers/clk/qcom/gcc-msm8960.c > >>>b/drivers/clk/qcom/gcc-msm8960.c > >>>index eb6a4f9..2c80d03 100644 > >>>--- a/drivers/clk/qcom/gcc-msm8960.c > >>>+++ b/drivers/clk/qcom/gcc-msm8960.c > >>>@@ -15,6 +15,7 @@ > >>> #include <linux/bitops.h> > >>> #include <linux/err.h> > >>> #include <linux/platform_device.h> > >>>+#include <linux/of_platform.h> > >>> #include <linux/module.h> > >>> #include <linux/of.h> > >>> #include <linux/of_device.h> > >>>@@ -3520,7 +3521,8 @@ static int gcc_msm8960_probe(struct > >>>platform_device *pdev) > >>> if (IS_ERR(clk)) > >>> return PTR_ERR(clk); > >>> > >>>- return qcom_cc_probe(pdev, match->data); > >>>+ qcom_cc_probe(pdev, match->data); > >>>+ return of_platform_populate(pdev->dev.of_node, NULL, NULL, > >>>&pdev->dev); > >> > >>We just lost the error code from qcom_cc_probe()... > >> > >I think Rajendra picked up the wrong patch for this series, I did submit > >a v2 (https://patches.linaro.org/44033/) with the above fixed. > > ah, sorry about that. > > >>Also, I don't like having a subnode in DT. Why can't we use the > >>same node as the GCC node and create a virtual child device here > >>for tsens? We can assign the same of_node that this platform > >>device has so that DT keeps working correctly. > > So the current driver looks up data based on compatible strings. The tsens device is always the same piece of hardware. The only thing that's changing is the qfprom data and the number of sensors. So we should be looking at the qfprom compatible string to figure out how to interpret the qfprom data which would include the number of sensors and how the data is encoded. > So you suggesting to create a virtual child device for gcc and > associate the gcc DT node to it? (And have the tsens compatible > mentioned as part of the gcc DT node?) No. The driver should work just fine without having to interrogate the device's compatible string. If we still need the compatible check for some reason, then we can always match based on qcom,gcc-msm8960, qcom,gcc-apq8064, etc. But I don't see why we need to do that when we should be looking at what type of qfprom is connected so we can correctly parse the data. > How is this any different from creating a subnode in DT anyway? The difference is we don't make up nodes to satisfy linux device driver design. I suspect the hardware engineers put tsens inside gcc because both blocks were written by the same person/team and they just needed some place to jam some registers and call it a day. That doesn't constitute an MFD or bus, which is what we would be expressing in DT if we made a child node, it constitutes a horrible software interface design that we have to live with. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
