Otavio Salvador <otavio@xxxxxxxxxxxxxxxx> wrote:
>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
Firstly imx-adc is not going to work. Imx6-adc would be fine. If you go with simply Imx-adc you are implying it will work with ALL
imx ADC IP. That is already untrue as this is the second driver for an ADC on an imx chip. Secondly convention is that naming goes
with the first supported part. Having said that I don't care that strongly if everyone wants imx6-adc.
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
--
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
