From: Otavio Salvador <otavio@xxxxxxxxxxxxxxxx>
Data: Wednesday, November 27, 2013 9:37 AM
>To: Duan Fugang-B38611
>Cc: Shawn Guo; jic23@xxxxxxxxxx; sachin.kamat@xxxxxxxxxx; Li Frank-B20596;
>linux-iio@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH v2 2/3] iio:adc:imx: add Freescale Vybrid vf610 adc driver
>
>On Tue, Nov 26, 2013 at 11:34 PM, Fugang Duan <fugang.duan@xxxxxxxxxxxxx> wrote:
>> From: Otavio Salvador <otavio@xxxxxxxxxxxxxxxx>
>> Data: Wednesday, November 27, 2013 8:17 AM
>>
>>>To: Shawn Guo
>>>Cc: Duan Fugang-B38611; jic23@xxxxxxxxxx; sachin.kamat@xxxxxxxxxx; Li
>>>Frank- B20596; linux-iio@xxxxxxxxxxxxxxx
>>>Subject: Re: [PATCH v2 2/3] iio:adc:imx: add Freescale Vybrid vf610
>>>adc driver
>>>
>>>On Tue, Nov 26, 2013 at 6:03 AM, Shawn Guo <shawn.guo@xxxxxxxxxx> wrote:
>>>> On Tue, Nov 26, 2013 at 07:33:11AM +0000, Fugang Duan wrote:
>>>>> >> +static const struct of_device_id vf610_adc_match[] = {
>>>>> >> + { .compatible = "fsl,vf610-adc", .data = (void *)VF610_ADC,
>>>>> >> +},
>>>>> >
>>>>> >We should .data for now, since it's currently unused.
>>>>>
>>>>> Ok, I will change it as below.
>>>>> { .compatible = "fsl,vf610-adc", .data = (void *)0, },
>>>>
>>>> The following is enough.
>>>>
>>>> { .compatible = "fsl,vf610-adc", }
>>>>
>>>> <snip>
>>>>
>>>>> >> +static int vf610_adc_probe(struct platform_device *pdev) {
>>>>> >> + struct vf610_adc *info = NULL;
>>>>> >> + struct device_node *np = pdev->dev.of_node;
>>>>> >> + struct iio_dev *indio_dev;
>>>>> >> + struct resource *mem;
>>>>> >> + int ret = -ENODEV;
>>>>> >> +
>>>>> >> + if (!np)
>>>>> >> + return ret;
>>>>> >
>>>>> >Unnecessary check. We will not be here at all if np is NULL.
>>>>>
>>>>> Great. There still have many drivers do it like this.
>>>>
>>>> It's unnecessary for platforms that only support DT probe, like
>>>> VF610 and IMX6.
>>>
>>>I know this is not my call but I would prefer it had a imx-adc and in
>>>the Kconfig help you explain clearly which SoCs it will/support. It
>>>will be strange to have i.MX6SLX using a VF610 ADC :-(
>>
>> i.MX6SLX ADC IP inherit VF610 ADC IP. The VF610 ADC support hardware trigger
>from PDB (The Programmable Delay Block), but i.MX6SLX don't support hardware
>trigger.
>> So, for the driver feature expander, there have difference for VF610 and
>i.MX6SLX. VF610 use the compatible "vf610-adc", i.MX6SLX use "imx-adc".
>
>I see; but the source code will be vf610_adc.c; I'd expect it to be imx_adc.c
>
Yes, the ADC patch set V1 version name the driver after imx_adc.c, but Jonathan Cameron think it is generic and suggest to name it one silicon name.
Thanks,
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
