[]..
+Example: +tsens: thermal-sensor@900000 { + compatible = "qcom,msm8916-tsens"; + qcom,tsens-slopes = <1176 1176 1154 1176 1111 + 1132 1132 1199 1132 1199 + 1132>; + nvmem-cells = <&tsens_caldata>, <&tsens_calsel>; + nvmem-cell-names = "caldata", "calsel"; + qcom,tsens-slopes = <3200 3200 3200 3200 3200>; + qcom,sensor-id = <0 1 2 4 5>; + #thermal-sensor-cells = <1>; + };The qcom,tsens-slopes property appears twice in the above example.
sure, will fix.
[...]+#ifdef CONFIG_PM +static int tsens_suspend(struct device *dev) +{ + struct tsens_device *tmdev = dev_get_drvdata(dev); + + if (tmdev->ops && tmdev->ops->suspend) + return tmdev->ops->suspend(tmdev); + + return 0; +} + +static int tsens_resume(struct device *dev) +{ + struct tsens_device *tmdev = dev_get_drvdata(dev); + + if (tmdev->ops && tmdev->ops->resume) + return tmdev->ops->resume(tmdev); + + return 0; +} + +static SIMPLE_DEV_PM_OPS(tsens_pm_ops, tsens_suspend, tsens_resume); +#define TSENS_PM_OPS (&tsens_pm_ops) + +#else /* CONFIG_PM_SLEEP */^+#define TSENS_PM_OPS NULL +#endif /* CONFIG_PM_SLEEP */^ The comments don't match the #ifdef above. I maybe wrong but looking at other drivers it looks like you don't need the #else part. You should be able to use the tsens_pm_ops without having to set it to NULL.
yes, I should be able to avoid the #else and thanks for catching the mismatched comments.
+ +static const struct of_device_id tsens_table[] = { + { + .compatible = "qcom,msm8916-tsens", + }, { + .compatible = "qcom,msm8974-tsens", + }, + {} +}; +MODULE_DEVICE_TABLE(of, tsens_table); + +static const struct thermal_zone_of_device_ops tsens_of_ops = { + .get_temp = tsens_get_temp, + .get_trend = tsens_get_trend, +}; + +static int tsens_register(struct tsens_device *tmdev) +{ + int i, ret; + struct thermal_zone_device *tzd; + u32 *hw_id, n = tmdev->num_sensors; + struct device_node *np = tmdev->dev->of_node; + + hw_id = devm_kcalloc(tmdev->dev, n, sizeof(u32), GFP_KERNEL); + if (!hw_id) + return -ENOMEM; + + ret = of_property_read_u32_array(np, "qcom,sensor-id", hw_id, n); + if (ret) + for (i = 0; i < tmdev->num_sensors; i++) + tmdev->sensor[i].hw_id = i; + else + for (i = 0; i < tmdev->num_sensors; i++) + tmdev->sensor[i].hw_id = hw_id[i]; +You could move the check for vaild for valid sensor ids in the device tree (ret) inside a single for loop. In that case the loop above could be merged into the iteration over the sensors below.
sure, seems like a reasonable optimization to avoid a few loops.
+ for (i = 0; i < tmdev->num_sensors; i++) { + tmdev->sensor[i].tmdev = tmdev; + tmdev->sensor[i].id = i; + tzd = thermal_zone_of_sensor_register(tmdev->dev, i, + &tmdev->sensor[i], + &tsens_of_ops); + if (IS_ERR(tzd)) + continue; + tmdev->sensor[i].tzd = tzd; + if (tmdev->ops->enable) + tmdev->ops->enable(tmdev, i); + } + return 0; +} + +static int tsens_probe(struct platform_device *pdev) +{ + int ret, i, num; + struct device *dev; + struct device_node *np; + struct tsens_sensor *s; + struct tsens_device *tmdev; + const struct of_device_id *id; + + dev = &pdev->dev; + np = dev->of_node;These assignments can be done with the declaration above.
I just have these assignments done conditionally in later patches (5/9), so left them here. Thanks for taking time to review. regards, Rajendra -- Qualcomm Innovation Center, Inc. is a member of 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
