On Tue, Nov 26, 2013 at 10:56:33AM +0000, Fugang Duan wrote:
> Add Freescale Vybrid vf610 adc driver. The driver only support
> ADC software trigger.
>
> Signed-off-by: Fugang Duan <B38611@xxxxxxxxxxxxx>
> ---
> drivers/iio/adc/Kconfig | 11 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/vf610_adc.c | 744 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 756 insertions(+), 0 deletions(-)
[...]
> +static void vf610_adc_cfg_of_init(struct vf610_adc *info)
> +{
> + struct device_node *np = info->dev->of_node;
> +
> + /*
> + * set default Configuration for ADC controller
> + * This config may upgrade to require from DT
> + */
> + info->adc_feature.clk_sel = ADCIOC_BUSCLK_SET;
> + info->adc_feature.vol_ref = VF610_ADCIOC_VR_VREF_SET;
> + info->adc_feature.calibration = true;
> + info->adc_feature.tri_hw = false;
> + info->adc_feature.dataov_en = true;
> + info->adc_feature.ac_clk_en = false;
> + info->adc_feature.dma_en = false;
> + info->adc_feature.cc_en = false;
> + info->adc_feature.cmp_func_en = false;
> + info->adc_feature.cmp_range_en = false;
> + info->adc_feature.cmp_greater_en = false;
> +
> + if (of_property_read_u32(np, "fsl,adc-io-pinctl",
> + &info->adc_feature.pctl)) {
> + info->adc_feature.pctl = VF610_ADC_IOPCTL5;
> + dev_info(info->dev,
> + "Miss adc-io-pinctl in dt, enable pin SE5.\n");
> + }
Could you elaborate on what this means? What is this doing when pinctrl
is not specified in the DT?
> +
> + if (info->vref)
> + info->vref_uv = regulator_get_voltage(info->vref);
> + else if (of_property_read_u32(np, "fsl,adc-vref", &info->vref_uv))
> + dev_err(info->dev,
> + "Miss adc-vref property or vref regulator in the DT.\n");
Query the regulator instead.
> +
> + if (of_property_read_u32(np, "fsl,adc-clk-div",
> + &info->adc_feature.clk_div_num)) {
> + info->adc_feature.clk_div_num = 2;
> + dev_info(info->dev,
> + "Miss adc-clk-div in dt, set divider to 2.\n");
> + }
Why do you need this?
I'd expect the use to request a frequency they wanted to sample at, and
the driver would try its best by fiddling with this and the clock input.
> +
> + if (of_property_read_u32(np, "fsl,adc-res",
> + &info->adc_feature.res_mode)) {
> + info->adc_feature.res_mode = 12;
> + dev_info(info->dev,
> + "Miss adc-res property in dt, use 8bit mode.\n");
This message does not seem to match the code above it.
Is this a hardware limitation in some instances? Or is it always
possible to select any of the resolutions?
> +
> + if (of_property_read_u32(np, "fsl,adc-sam-time",
> + &info->adc_feature.sam_time)) {
> + info->adc_feature.sam_time = 4;
> + dev_info(info->dev,
> + "Miss adc-sam-time property in dt, set to 4.\n");
> + }
This looks like it should be handled at runtime.
> +
> + info->adc_feature.hw_average = of_property_read_bool(np,
> + "fsl,adc-hw-aver-en");
Is hw average support not always present?
> + if (info->adc_feature.hw_average &&
> + of_property_read_u32(np, "fsl,adc-aver-sam-sel",
> + &info->adc_feature.hw_sam)) {
> + info->adc_feature.hw_sam = 4;
> + dev_info(info->dev,
> + "Miss adc-aver-sam-sel property in dt, set to 4.\n");
> + }
Why not configure this at runtime instead?
> +
> + info->adc_feature.lpm = of_property_read_bool(np, "fsl,adc-low-power-mode");
> + info->adc_feature.hs_oper = of_property_read_bool(np, "fsl,adc-high-speed-conv");
Please elaborate on what these mean, and why they need to be in the DT.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
