Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-02-11 Thread Olof Johansson
Hi,

It looks like the bindings here are still completely unsettled, or at least
unimplemented -- there seems to be a preference for an encoding similar to how
interrupts/gpios/clks work. I think that wouldn't be a bad idea.

For the case of this particular driver and connections, it'd just mean you
would need to do the binding slightly differently. You can still have some of
the sensors under the ADC node if they're not sitting on another bus where it
makes sense to locate them.


Some further comments below.


On Thu, Jan 24, 2013 at 10:35:32AM +0530, Naveen Krishna Chatradhi wrote:
> This patch add an ADC IP found on EXYNOS5250 and EXYNOS5410 SoCs
> from Samsung. Also adds the Documentation for device tree bindings.
> 
> Signed-off-by: Naveen Krishna Chatradhi 
> ---
> Changes since v1:
> 
> 1. Fixed comments from Lars
> 2. Added support for ADC on EXYNOS5410
> 
> Changes since v2:
> 
> 1. Changed the instance name for (struct iio_dev *) to indio_dev
> 2. Changed devm_request_irq to request_irq
> 
> Few doubts regarding the mappings and child device handling.
> Kindly, suggest me better methods.
> 
> Changes since v3:
> 
> 1. Added clk_prepare_disable and regulator_disable calls in _remove()
> 2. Moved init_completion before irq_request
> 3. Added NULL pointer check for devm_request_and_ioremap() return value.
> 4. Use number of channels as per the ADC version 
> 5. Change the define ADC_V1_CHANNEL to ADC_CHANNEL
> 6. Update the Documentation to include EXYNOS5410 compatible
> 
> Changes since v4:
> 
> 1. if devm_request_and_ioremap() failes, free iio_device before returning 
> 
> Doug, i've used
>   chan = iio_channel_get(dev_name(&pdev->dev), "adc3");
> in ntc thermistor driver during probe and
>   iio_read_channel_raw(chan, &val);
> for read.
> 
> But, then the drivers become kind of coupled. Right.
> 
> Lars, Is there an other way.
> 
> Thanks for the comments
> 
>  .../bindings/arm/samsung/exynos5-adc.txt   |   38 ++
>  drivers/iio/adc/Kconfig|7 +
>  drivers/iio/adc/Makefile   |1 +
>  drivers/iio/adc/exynos5_adc.c  |  477 
> 
>  4 files changed, 523 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
>  create mode 100644 drivers/iio/adc/exynos5_adc.c
> 
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt 
> b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
> new file mode 100644
> index 000..0f281d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
> @@ -0,0 +1,38 @@
> +Samsung Exynos5 Analog to Digital Converter bindings
> +
> +Required properties:
> +- compatible:Must be "samsung,exynos5250-adc" for exynos5250 
> controllers.
> + Must be "samsung,exynos5410-adc" for exynos5410 
> controllers.

Looking at the out-of-tree patch we have that adds exynos5 support to
arch/arm/plat-samsung/adc.c, this looks like it should be:

samsung,exynos4412-adc should be the base case (this was the first chip with
the new "version 4" ADC hardware). Then for 5250 the chip-specific
prefixes should also be there for compatible in case there are SoC-specific
bugs or quirks or features that the driver in the future might need to care
about.

Based on the code below, it looks like 5410 isn't compatible with 4412, since
it needs special init sequences. So that means 5410 shouldn't be claimed to be
compatible.


> +- reg:   Contains ADC register address range (base 
> address and
> + length).
> +- interrupts:Contains the interrupt information for the 
> timer. The
> + format is being dependent on which interrupt controller
> + the Samsung device uses.
> +
> +Note: child nodes can be added for auto probing from device tree.
> +
> +Example: adding device info in dtsi file
> +
> +adc@12D1 {
> + compatible = "samsung,exynos5250-adc";
> + reg = <0x12D1 0x100>;
> + interrupts = <0 106 0>;
> + #address-cells = <1>;
> + #size-cells = <1>;

This is where something like #channel-cells or something else would be needed
-- not address cells and size cells. And then (see below)...

> + ranges;
> +};
> +
> +
> +Example: Adding child nodes in dts file
> +
> +adc@12D1 {
> +
> + /* NTC thermistor is a hwmon device */
> + ncp15wb473@0 {
> + compatible = "ntc,ncp15wb473";
> + reg = <0x0>;
> + pullup-uV = <180>;
> + pullup-ohm = <47000>;
> + pulldown-ohm = <0>;

So, here is where the reference to channel should be specified. Even for those
who are located as children to the adc node.

Something like (naming is up in the air):
io-channel = <&adc 0>;

> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index fe822a1..33ceabf 100644
> --- a/drivers/iio/a

Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-29 Thread Naveen Krishna Ch
On 26 January 2013 16:27, Jonathan Cameron  wrote:
> On 01/24/2013 04:58 AM, Naveen Krishna Chatradhi wrote:
>> This patch adds driver for ADC IP found on EXYNOS5250 and EXYNOS5410
>> from Samsung. Also adds the Documentation for device tree bindings.
>>
>> Signed-off-by: Naveen Krishna Chatradhi 
> Just a quick general comment on patch formatting. For later versions give
> a title of
> [PATCH V5]...
> to the email as then it's easy for those of us who have been sitting
> back and quitely not reading the thread to figure out which the latest
> patch is.
>
> Thanks
>
> Jonathan
Sure Jonathan (I was assuming using message id would be enough), i
will correct it.
>
>> ---
>> Changes since v1:
>>
>> 1. Fixed comments from Lars
>> 2. Added support for ADC on EXYNOS5410
>>
>> Changes since v2:
>>
>> 1. Changed the instance name for (struct iio_dev *) to indio_dev
>> 2. Changed devm_request_irq to request_irq
>>
>> Few doubts regarding the mappings and child device handling.
>> Kindly, suggest me better methods.
>>
>> Changes since v3:
>>
>> 1. Added clk_prepare_disable and regulator_disable calls in _remove()
>> 2. Moved init_completion before irq_request
>> 3. Added NULL pointer check for devm_request_and_ioremap() return value.
>> 4. Use number of channels as per the ADC version
>> 5. Change the define ADC_V1_CHANNEL to ADC_CHANNEL
>> 6. Update the Documentation to include EXYNOS5410 compatible
>>
>> Doug, i've used
>>   chan = iio_channel_get(dev_name(&pdev->dev), "adc3");
>> in ntc thermistor driver during probe and
>>   iio_read_channel_raw(chan, &val);
>> for read.
>>
>> But, then the drivers become kind of coupled. Right.
>>
>> Lars, Is there an other way.
>>
>> Thanks for the comments
>>
>>  .../bindings/arm/samsung/exynos5-adc.txt   |   38 ++
>>  drivers/iio/adc/Kconfig|7 +
>>  drivers/iio/adc/Makefile   |1 +
>>  drivers/iio/adc/exynos5_adc.c  |  475 
>> 
>>  4 files changed, 521 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
>>  create mode 100644 drivers/iio/adc/exynos5_adc.c
>>
>> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt 
>> b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
>> new file mode 100644
>> index 000..0f281d9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
>> @@ -0,0 +1,38 @@
>> +Samsung Exynos5 Analog to Digital Converter bindings
>> +
>> +Required properties:
>> +- compatible:Must be "samsung,exynos5250-adc" for 
>> exynos5250 controllers.
>> + Must be "samsung,exynos5410-adc" for exynos5410 
>> controllers.
>> +- reg:   Contains ADC register address range (base 
>> address and
>> + length).
>> +- interrupts:Contains the interrupt information for the 
>> timer. The
>> + format is being dependent on which interrupt controller
>> + the Samsung device uses.
>> +
>> +Note: child nodes can be added for auto probing from device tree.
>> +
>> +Example: adding device info in dtsi file
>> +
>> +adc@12D1 {
>> + compatible = "samsung,exynos5250-adc";
>> + reg = <0x12D1 0x100>;
>> + interrupts = <0 106 0>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> +};
>> +
>> +
>> +Example: Adding child nodes in dts file
>> +
>> +adc@12D1 {
>> +
>> + /* NTC thermistor is a hwmon device */
>> + ncp15wb473@0 {
>> + compatible = "ntc,ncp15wb473";
>> + reg = <0x0>;
>> + pullup-uV = <180>;
>> + pullup-ohm = <47000>;
>> + pulldown-ohm = <0>;
>> + };
>> +};
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index fe822a1..33ceabf 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -91,6 +91,13 @@ config AT91_ADC
>>   help
>> Say yes here to build support for Atmel AT91 ADC.
>>
>> +config EXYNOS5_ADC
>> + bool "Exynos5 ADC driver support"
>> + help
>> +   Core support for the ADC block found in the Samsung EXYNOS5 series
>> +   of SoCs for drivers such as the touchscreen and hwmon to use to share
>> +   this resource.
>> +
>>  config LP8788_ADC
>>   bool "LP8788 ADC driver"
>>   depends on MFD_LP8788
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> index 2d5f100..5b4a4f6 100644
>> --- a/drivers/iio/adc/Makefile
>> +++ b/drivers/iio/adc/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
>>  obj-$(CONFIG_AD7793) += ad7793.o
>>  obj-$(CONFIG_AD7887) += ad7887.o
>>  obj-$(CONFIG_AT91_ADC) += at91_adc.o
>> +obj-$(CONFIG_EXYNOS5_ADC) += exynos5_adc.o
>>  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>>  obj-$(CONFIG_MAX1363) += max1363.o
>>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>> diff --git a

Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-26 Thread Jonathan Cameron
On 01/24/2013 04:58 AM, Naveen Krishna Chatradhi wrote:
> This patch adds driver for ADC IP found on EXYNOS5250 and EXYNOS5410
> from Samsung. Also adds the Documentation for device tree bindings.
> 
> Signed-off-by: Naveen Krishna Chatradhi 
Just a quick general comment on patch formatting. For later versions give
a title of
[PATCH V5]...
to the email as then it's easy for those of us who have been sitting
back and quitely not reading the thread to figure out which the latest
patch is.

Thanks

Jonathan

> ---
> Changes since v1:
> 
> 1. Fixed comments from Lars
> 2. Added support for ADC on EXYNOS5410
> 
> Changes since v2:
> 
> 1. Changed the instance name for (struct iio_dev *) to indio_dev
> 2. Changed devm_request_irq to request_irq
> 
> Few doubts regarding the mappings and child device handling.
> Kindly, suggest me better methods.
> 
> Changes since v3:
> 
> 1. Added clk_prepare_disable and regulator_disable calls in _remove()
> 2. Moved init_completion before irq_request
> 3. Added NULL pointer check for devm_request_and_ioremap() return value.
> 4. Use number of channels as per the ADC version 
> 5. Change the define ADC_V1_CHANNEL to ADC_CHANNEL
> 6. Update the Documentation to include EXYNOS5410 compatible
> 
> Doug, i've used
>   chan = iio_channel_get(dev_name(&pdev->dev), "adc3");
> in ntc thermistor driver during probe and
>   iio_read_channel_raw(chan, &val);
> for read.
> 
> But, then the drivers become kind of coupled. Right.
> 
> Lars, Is there an other way.
> 
> Thanks for the comments
> 
>  .../bindings/arm/samsung/exynos5-adc.txt   |   38 ++
>  drivers/iio/adc/Kconfig|7 +
>  drivers/iio/adc/Makefile   |1 +
>  drivers/iio/adc/exynos5_adc.c  |  475 
> 
>  4 files changed, 521 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
>  create mode 100644 drivers/iio/adc/exynos5_adc.c
> 
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt 
> b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
> new file mode 100644
> index 000..0f281d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
> @@ -0,0 +1,38 @@
> +Samsung Exynos5 Analog to Digital Converter bindings
> +
> +Required properties:
> +- compatible:Must be "samsung,exynos5250-adc" for exynos5250 
> controllers.
> + Must be "samsung,exynos5410-adc" for exynos5410 
> controllers.
> +- reg:   Contains ADC register address range (base 
> address and
> + length).
> +- interrupts:Contains the interrupt information for the 
> timer. The
> + format is being dependent on which interrupt controller
> + the Samsung device uses.
> +
> +Note: child nodes can be added for auto probing from device tree.
> +
> +Example: adding device info in dtsi file
> +
> +adc@12D1 {
> + compatible = "samsung,exynos5250-adc";
> + reg = <0x12D1 0x100>;
> + interrupts = <0 106 0>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +};
> +
> +
> +Example: Adding child nodes in dts file
> +
> +adc@12D1 {
> +
> + /* NTC thermistor is a hwmon device */
> + ncp15wb473@0 {
> + compatible = "ntc,ncp15wb473";
> + reg = <0x0>;
> + pullup-uV = <180>;
> + pullup-ohm = <47000>;
> + pulldown-ohm = <0>;
> + };
> +};
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index fe822a1..33ceabf 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -91,6 +91,13 @@ config AT91_ADC
>   help
> Say yes here to build support for Atmel AT91 ADC.
>  
> +config EXYNOS5_ADC
> + bool "Exynos5 ADC driver support"
> + help
> +   Core support for the ADC block found in the Samsung EXYNOS5 series
> +   of SoCs for drivers such as the touchscreen and hwmon to use to share
> +   this resource.
> +
>  config LP8788_ADC
>   bool "LP8788 ADC driver"
>   depends on MFD_LP8788
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 2d5f100..5b4a4f6 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
>  obj-$(CONFIG_AD7793) += ad7793.o
>  obj-$(CONFIG_AD7887) += ad7887.o
>  obj-$(CONFIG_AT91_ADC) += at91_adc.o
> +obj-$(CONFIG_EXYNOS5_ADC) += exynos5_adc.o
>  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> diff --git a/drivers/iio/adc/exynos5_adc.c b/drivers/iio/adc/exynos5_adc.c
> new file mode 100644
> index 000..4963649
> --- /dev/null
> +++ b/drivers/iio/adc/exynos5_adc.c
> @@ -0,0 +1,475 @@
> +/*
> + *  exynos5_adc.c - Support for ADC in EXYNOS5 SoCs
> + *
> + *  8 ~ 10 cha

Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-24 Thread Lars-Peter Clausen
On 01/24/2013 08:15 PM, Tomasz Figa wrote:
> Hi,
> 
> On Thursday 24 of January 2013 19:19:57 Lars-Peter Clausen wrote:
>> On 01/24/2013 05:12 PM, Doug Anderson wrote:
>>> Lars,
>>>
>>> Thank you for your comments / thoughts...
>>
>> Hi,
>>
>>> On Thu, Jan 24, 2013 at 1:54 AM, Lars-Peter Clausen  
> wrote:
 adc: adc@12D1 {

 #io-channel-cells = <1>;
 io-channel-output-names = "adc1", "adc2", ...;
 
 ncp15wb473@0 {
 
 compatible = "ntc,ncp15wb473";
 ...
 io-channels = <&adc 0>; // First ADC channel
>>>
>>> I'm not an expert, but I think the typical way is:
>>> * No need to include a handle to &adc.  It's logically our parent.  In
>>> a similar way i2c devices don't specify their parent bus--they are
>>> just listed under it.
>>> * The "0" should be specified with reg = <0>
>>
>> The relationship between the IIO sensor device and the consumer device
>> is not always a parent child relationship. In this case it makes sense
>> to have the ADC as the parent for the thermistors. But for other cases
>> this may not be true. E.g. take a touchscreen or power monitoring
>> platform device which uses the IIO device to do measurements.
> 
> The policy is to use children with reg property only inside a node 
> representing a bus controller through which the child device is being 
> accessed (like I2C, SPI).
> 
> I would see IIO bindings similar to what we have with GPIOs, interrupts or 
> regulators, so io-channels = <&iio-controller channel> seems fine (or 
> rather iio-channels) with the node under appropriate parent.

IIO is a very Linux specific term, the device tree bindings should be as OS
agnostic as possible, so io-channels is probably the better term.


> 
>>> To implement this I'd imagine that we'll need a new API call, right?
>>> In this case the thermistor driver won't know the name of the channel.
>>>
>>>  It can find the ADC (the struct device and probably other things) and
>>>
>>> knows a channel index.  Am I understanding properly?
>>
>> This can be done by adding a new api call, but it would be best if both
>> dt and non-dt based consumers can use the same function. I outlined one
>> possible solution how this can be done in the previous mail to Naveen.
> 
> In case of the solution I mentioned, implementation would be almost 
> identical to what is done with GPIOs (see drivers/gpio/gpiolib-of.c).

Although similar to the GPIO bindings, the clk bindings are in my opinion an
even better example.

- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-24 Thread Tomasz Figa
Hi,

On Thursday 24 of January 2013 19:19:57 Lars-Peter Clausen wrote:
> On 01/24/2013 05:12 PM, Doug Anderson wrote:
> > Lars,
> > 
> > Thank you for your comments / thoughts...
> 
> Hi,
> 
> > On Thu, Jan 24, 2013 at 1:54 AM, Lars-Peter Clausen  
wrote:
> >> adc: adc@12D1 {
> >> 
> >> #io-channel-cells = <1>;
> >> io-channel-output-names = "adc1", "adc2", ...;
> >> 
> >> ncp15wb473@0 {
> >> 
> >> compatible = "ntc,ncp15wb473";
> >> ...
> >> io-channels = <&adc 0>; // First ADC channel
> > 
> > I'm not an expert, but I think the typical way is:
> > * No need to include a handle to &adc.  It's logically our parent.  In
> > a similar way i2c devices don't specify their parent bus--they are
> > just listed under it.
> > * The "0" should be specified with reg = <0>
> 
> The relationship between the IIO sensor device and the consumer device
> is not always a parent child relationship. In this case it makes sense
> to have the ADC as the parent for the thermistors. But for other cases
> this may not be true. E.g. take a touchscreen or power monitoring
> platform device which uses the IIO device to do measurements.

The policy is to use children with reg property only inside a node 
representing a bus controller through which the child device is being 
accessed (like I2C, SPI).

I would see IIO bindings similar to what we have with GPIOs, interrupts or 
regulators, so io-channels = <&iio-controller channel> seems fine (or 
rather iio-channels) with the node under appropriate parent.

> > To implement this I'd imagine that we'll need a new API call, right?
> > In this case the thermistor driver won't know the name of the channel.
> > 
> >  It can find the ADC (the struct device and probably other things) and
> > 
> > knows a channel index.  Am I understanding properly?
> 
> This can be done by adding a new api call, but it would be best if both
> dt and non-dt based consumers can use the same function. I outlined one
> possible solution how this can be done in the previous mail to Naveen.

In case of the solution I mentioned, implementation would be almost 
identical to what is done with GPIOs (see drivers/gpio/gpiolib-of.c).

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-24 Thread Lars-Peter Clausen
On 01/24/2013 05:12 PM, Doug Anderson wrote:
> Lars,
> 
> Thank you for your comments / thoughts...
> 

Hi,

> 
> On Thu, Jan 24, 2013 at 1:54 AM, Lars-Peter Clausen  wrote:
>> adc: adc@12D1 {
>>
>> #io-channel-cells = <1>;
>> io-channel-output-names = "adc1", "adc2", ...;
>>
>> ncp15wb473@0 {
>> compatible = "ntc,ncp15wb473";
>> ...
>> io-channels = <&adc 0>; // First ADC channel
> 
> I'm not an expert, but I think the typical way is:
> * No need to include a handle to &adc.  It's logically our parent.  In
> a similar way i2c devices don't specify their parent bus--they are
> just listed under it.
> * The "0" should be specified with reg = <0>

The relationship between the IIO sensor device and the consumer device is
not always a parent child relationship. In this case it makes sense to have
the ADC as the parent for the thermistors. But for other cases this may not
be true. E.g. take a touchscreen or power monitoring platform device which
uses the IIO device to do measurements.

> 
> To implement this I'd imagine that we'll need a new API call, right?
> In this case the thermistor driver won't know the name of the channel.
>  It can find the ADC (the struct device and probably other things) and
> knows a channel index.  Am I understanding properly?

This can be done by adding a new api call, but it would be best if both dt
and non-dt based consumers can use the same function. I outlined one
possible solution how this can be done in the previous mail to Naveen.

- Lars

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-24 Thread Lars-Peter Clausen
On 01/24/2013 03:20 PM, Naveen Krishna Ch wrote:
> On 24 January 2013 15:24, Lars-Peter Clausen  wrote:
>>
>> On 01/24/2013 01:42 AM, Doug Anderson wrote:
>>> Lars,
>>>
>>> On Wed, Jan 23, 2013 at 4:52 AM, Lars-Peter Clausen  wrote:
> Few doubts regarding the mappings and child device handling.
> Kindly, suggest me better methods.

 The patch looks mostly good now. As for the mappings, the problem is that 
 we
 currently do not have any device tree bindings for IIO. So a proper 
 solution
 would be to add dt bindings for IIO.
>>>
>>> Can you explain more how you think this would work?  It seems like
>>> just having child nodes as platform drivers would be OK (to me) and
>>> having them instantiated with of_platform_populate() seems reasonable.
>>>
>>> ...but the one thing that is missing is a way for children to get
>>> access to the channel properly.
>>>
>>> The children have access to the ADC "struct device" (it is their
>>> parent device) and have a channel number (their "reg" field), but
>>> there is no API call to map this to a "struct iio_channel".  Is that
>>> all that's needed in this case?  ...or are you envisioning something
>>> more?
>>
>> Well, the idea is that the consumer doesn't need to know the channel number.
>> That's what the mapping takes care of. It basically creates a consumer alias
>> for the channel. When requesting the channel the consumer always uses the
>> same name.
>>
>> iio_channel_get(dev_name(&pdev->dev), "voltage");
>>
>> And the mapping contains an entry which maps the tuple of device name and
>> channel name to a real IIO channel which as been registered by a IIO device.
>> Note if there is only one channel you can also just use NULL for the channel
>> name. This is similar to how clocks are managed with the clk framework.
>>
>> Now for the dt bindings I think we should stick to something similar to what
>> the clk framework does.
>>
>> E.g.
>>
>> adc: adc@12D1 {
>>
>> #io-channel-cells = <1>;
>> io-channel-output-names = "adc1", "adc2", ...;
>>
>> ncp15wb473@0 {
>> compatible = "ntc,ncp15wb473";
>> ...
>> io-channels = <&adc 0>; // First ADC channel
>> io-channel-names = "voltage";
>> };
>>
>> ncp15wb473@0 {
>> compatible = "ntc,ncp15wb473";
>> ...
>> io-channels = <&adc 1>; // Second ADC channel
>> io-channel-names = "voltage";
>> }
>> };
>>
> Hello Lars,
> 
> I've a doubt here
> 
> How do i find the child dev_name for iio_map_array_register();
> 
> cause the child devices are probed during of_platform_populate();
> 
> and during the probe the child calls
> iio_channel_get(dev_name(&pdev->dev), "voltage");
> 
> The child device nodes are ncp15wb473.0 and ncp15wb473.1 in this case.
> But, this may change.


Hi,

I think we should handle the devicetree channel mapping in the IIO core and
not in the individual drivers. If we handle it in the core we do not need to
create a iio_map and won't need to know the name of the consumer.

You'd basically need to check whether the device requesting the IIO channel
has a of_node. If it has, check if it has an io-channels attribute. If it
also has an io-channels attribute lookup the IIO device by the phandle and
create a iio_channel for the nth channel of that device.

If either the device has no of_node or no io-channels attribute fallback to
using the iio_map based lookup method.

This would require one API change though, iio_channel_get would need to take
a device instead of the device name so it has access to both the device name
and the device node. Jonathan: any particular reason why you choose to let
iio_channel_get the device name instead of the device itself?

- Lars

> 
> Kindly, help.
> 
> Assume we have a device tree like this
> 
> adc@12D1 {
> #io-channel-cells = <1>;
> io-channel-output-names = "adc0", "adc1", "adc2",
> "adc3", "adc4", "adc5",
> "adc6", "adc7", "adc8", "adc9";
> 
> ncp15wb473@0 {
> compatible = "ntc,ncp15wb473";
> reg = <0x0>;
> io-channel-names = "voltage";
> pullup-uV = <180>;
> pullup-ohm = <47000>;
> pulldown-ohm = <0>;
> };
> ncp15wb473@1 {
> compatible = "ntc,ncp15wb473";
> reg = <0x1>;
> io-channel-names = "voltage";
> pullup-uV = <180>;
> pullup-ohm = <47000>;
> pulldown-ohm = <0>;
> };
> 
>   };
> 
> 
>> io-channel-output-names and io-channel-names can be optional. In the case
>> where there is only one channel it's 

Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-24 Thread Doug Anderson
Lars,

Thank you for your comments / thoughts...


On Thu, Jan 24, 2013 at 1:54 AM, Lars-Peter Clausen  wrote:
> adc: adc@12D1 {
>
> #io-channel-cells = <1>;
> io-channel-output-names = "adc1", "adc2", ...;
>
> ncp15wb473@0 {
> compatible = "ntc,ncp15wb473";
> ...
> io-channels = <&adc 0>; // First ADC channel

I'm not an expert, but I think the typical way is:
* No need to include a handle to &adc.  It's logically our parent.  In
a similar way i2c devices don't specify their parent bus--they are
just listed under it.
* The "0" should be specified with reg = <0>

Everything else about this syntax looks good.

To implement this I'd imagine that we'll need a new API call, right?
In this case the thermistor driver won't know the name of the channel.
 It can find the ADC (the struct device and probably other things) and
knows a channel index.  Am I understanding properly?

I think Naveen expressed the same question, though he said it a bit
differently than me.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-24 Thread Naveen Krishna Ch
On 24 January 2013 15:24, Lars-Peter Clausen  wrote:
>
> On 01/24/2013 01:42 AM, Doug Anderson wrote:
> > Lars,
> >
> > On Wed, Jan 23, 2013 at 4:52 AM, Lars-Peter Clausen  wrote:
> >>> Few doubts regarding the mappings and child device handling.
> >>> Kindly, suggest me better methods.
> >>
> >> The patch looks mostly good now. As for the mappings, the problem is that 
> >> we
> >> currently do not have any device tree bindings for IIO. So a proper 
> >> solution
> >> would be to add dt bindings for IIO.
> >
> > Can you explain more how you think this would work?  It seems like
> > just having child nodes as platform drivers would be OK (to me) and
> > having them instantiated with of_platform_populate() seems reasonable.
> >
> > ...but the one thing that is missing is a way for children to get
> > access to the channel properly.
> >
> > The children have access to the ADC "struct device" (it is their
> > parent device) and have a channel number (their "reg" field), but
> > there is no API call to map this to a "struct iio_channel".  Is that
> > all that's needed in this case?  ...or are you envisioning something
> > more?
>
> Well, the idea is that the consumer doesn't need to know the channel number.
> That's what the mapping takes care of. It basically creates a consumer alias
> for the channel. When requesting the channel the consumer always uses the
> same name.
>
> iio_channel_get(dev_name(&pdev->dev), "voltage");
>
> And the mapping contains an entry which maps the tuple of device name and
> channel name to a real IIO channel which as been registered by a IIO device.
> Note if there is only one channel you can also just use NULL for the channel
> name. This is similar to how clocks are managed with the clk framework.
>
> Now for the dt bindings I think we should stick to something similar to what
> the clk framework does.
>
> E.g.
>
> adc: adc@12D1 {
>
> #io-channel-cells = <1>;
> io-channel-output-names = "adc1", "adc2", ...;
>
> ncp15wb473@0 {
> compatible = "ntc,ncp15wb473";
> ...
> io-channels = <&adc 0>; // First ADC channel
> io-channel-names = "voltage";
> };
>
> ncp15wb473@0 {
> compatible = "ntc,ncp15wb473";
> ...
> io-channels = <&adc 1>; // Second ADC channel
> io-channel-names = "voltage";
> }
> };
>
Hello Lars,

I've a doubt here

How do i find the child dev_name for iio_map_array_register();

cause the child devices are probed during of_platform_populate();

and during the probe the child calls
iio_channel_get(dev_name(&pdev->dev), "voltage");

The child device nodes are ncp15wb473.0 and ncp15wb473.1 in this case.
But, this may change.

Kindly, help.

Assume we have a device tree like this

adc@12D1 {
#io-channel-cells = <1>;
io-channel-output-names = "adc0", "adc1", "adc2",
"adc3", "adc4", "adc5",
"adc6", "adc7", "adc8", "adc9";

ncp15wb473@0 {
compatible = "ntc,ncp15wb473";
reg = <0x0>;
io-channel-names = "voltage";
pullup-uV = <180>;
pullup-ohm = <47000>;
pulldown-ohm = <0>;
};
ncp15wb473@1 {
compatible = "ntc,ncp15wb473";
reg = <0x1>;
io-channel-names = "voltage";
pullup-uV = <180>;
pullup-ohm = <47000>;
pulldown-ohm = <0>;
};

  };


> io-channel-output-names and io-channel-names can be optional. In the case
> where there is only one channel it's not really necessary to use
> io-channel-names.
>
> - Lars




--
Shine bright,
(: Nav :)
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-24 Thread Lars-Peter Clausen
On 01/24/2013 01:42 AM, Doug Anderson wrote:
> Lars,
> 
> On Wed, Jan 23, 2013 at 4:52 AM, Lars-Peter Clausen  wrote:
>>> Few doubts regarding the mappings and child device handling.
>>> Kindly, suggest me better methods.
>>
>> The patch looks mostly good now. As for the mappings, the problem is that we
>> currently do not have any device tree bindings for IIO. So a proper solution
>> would be to add dt bindings for IIO.
> 
> Can you explain more how you think this would work?  It seems like
> just having child nodes as platform drivers would be OK (to me) and
> having them instantiated with of_platform_populate() seems reasonable.
> 
> ...but the one thing that is missing is a way for children to get
> access to the channel properly.
> 
> The children have access to the ADC "struct device" (it is their
> parent device) and have a channel number (their "reg" field), but
> there is no API call to map this to a "struct iio_channel".  Is that
> all that's needed in this case?  ...or are you envisioning something
> more?

Well, the idea is that the consumer doesn't need to know the channel number.
That's what the mapping takes care of. It basically creates a consumer alias
for the channel. When requesting the channel the consumer always uses the
same name.

iio_channel_get(dev_name(&pdev->dev), "voltage");

And the mapping contains an entry which maps the tuple of device name and
channel name to a real IIO channel which as been registered by a IIO device.
Note if there is only one channel you can also just use NULL for the channel
name. This is similar to how clocks are managed with the clk framework.

Now for the dt bindings I think we should stick to something similar to what
the clk framework does.

E.g.

adc: adc@12D1 {

#io-channel-cells = <1>;
io-channel-output-names = "adc1", "adc2", ...;

ncp15wb473@0 {
compatible = "ntc,ncp15wb473";
...
io-channels = <&adc 0>; // First ADC channel
io-channel-names = "voltage";
};

ncp15wb473@0 {
compatible = "ntc,ncp15wb473";
...
io-channels = <&adc 1>; // Second ADC channel
io-channel-names = "voltage";
}
};

io-channel-output-names and io-channel-names can be optional. In the case
where there is only one channel it's not really necessary to use
io-channel-names.

- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-23 Thread Naveen Krishna Chatradhi
This patch add an ADC IP found on EXYNOS5250 and EXYNOS5410 SoCs
from Samsung. Also adds the Documentation for device tree bindings.

Signed-off-by: Naveen Krishna Chatradhi 
---
Changes since v1:

1. Fixed comments from Lars
2. Added support for ADC on EXYNOS5410

Changes since v2:

1. Changed the instance name for (struct iio_dev *) to indio_dev
2. Changed devm_request_irq to request_irq

Few doubts regarding the mappings and child device handling.
Kindly, suggest me better methods.

Changes since v3:

1. Added clk_prepare_disable and regulator_disable calls in _remove()
2. Moved init_completion before irq_request
3. Added NULL pointer check for devm_request_and_ioremap() return value.
4. Use number of channels as per the ADC version 
5. Change the define ADC_V1_CHANNEL to ADC_CHANNEL
6. Update the Documentation to include EXYNOS5410 compatible

Changes since v4:

1. if devm_request_and_ioremap() failes, free iio_device before returning 

Doug, i've used
chan = iio_channel_get(dev_name(&pdev->dev), "adc3");
in ntc thermistor driver during probe and
iio_read_channel_raw(chan, &val);
for read.

But, then the drivers become kind of coupled. Right.

Lars, Is there an other way.

Thanks for the comments

 .../bindings/arm/samsung/exynos5-adc.txt   |   38 ++
 drivers/iio/adc/Kconfig|7 +
 drivers/iio/adc/Makefile   |1 +
 drivers/iio/adc/exynos5_adc.c  |  477 
 4 files changed, 523 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
 create mode 100644 drivers/iio/adc/exynos5_adc.c

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
new file mode 100644
index 000..0f281d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
@@ -0,0 +1,38 @@
+Samsung Exynos5 Analog to Digital Converter bindings
+
+Required properties:
+- compatible:  Must be "samsung,exynos5250-adc" for exynos5250 
controllers.
+   Must be "samsung,exynos5410-adc" for exynos5410 
controllers.
+- reg: Contains ADC register address range (base address and
+   length).
+- interrupts:  Contains the interrupt information for the timer. The
+   format is being dependent on which interrupt controller
+   the Samsung device uses.
+
+Note: child nodes can be added for auto probing from device tree.
+
+Example: adding device info in dtsi file
+
+adc@12D1 {
+   compatible = "samsung,exynos5250-adc";
+   reg = <0x12D1 0x100>;
+   interrupts = <0 106 0>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+};
+
+
+Example: Adding child nodes in dts file
+
+adc@12D1 {
+
+   /* NTC thermistor is a hwmon device */
+   ncp15wb473@0 {
+   compatible = "ntc,ncp15wb473";
+   reg = <0x0>;
+   pullup-uV = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   };
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index fe822a1..33ceabf 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -91,6 +91,13 @@ config AT91_ADC
help
  Say yes here to build support for Atmel AT91 ADC.
 
+config EXYNOS5_ADC
+   bool "Exynos5 ADC driver support"
+   help
+ Core support for the ADC block found in the Samsung EXYNOS5 series
+ of SoCs for drivers such as the touchscreen and hwmon to use to share
+ this resource.
+
 config LP8788_ADC
bool "LP8788 ADC driver"
depends on MFD_LP8788
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 2d5f100..5b4a4f6 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7793) += ad7793.o
 obj-$(CONFIG_AD7887) += ad7887.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
+obj-$(CONFIG_EXYNOS5_ADC) += exynos5_adc.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
diff --git a/drivers/iio/adc/exynos5_adc.c b/drivers/iio/adc/exynos5_adc.c
new file mode 100644
index 000..197d622
--- /dev/null
+++ b/drivers/iio/adc/exynos5_adc.c
@@ -0,0 +1,477 @@
+/*
+ *  exynos5_adc.c - Support for ADC in EXYNOS5 SoCs
+ *
+ *  8 ~ 10 channel, 10/12-bit ADC
+ *
+ *  Copyright (C) 2013 Naveen Krishna Chatradhi 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warra

[PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-23 Thread Naveen Krishna Chatradhi
This patch adds driver for ADC IP found on EXYNOS5250 and EXYNOS5410
from Samsung. Also adds the Documentation for device tree bindings.

Signed-off-by: Naveen Krishna Chatradhi 
---
Changes since v1:

1. Fixed comments from Lars
2. Added support for ADC on EXYNOS5410

Changes since v2:

1. Changed the instance name for (struct iio_dev *) to indio_dev
2. Changed devm_request_irq to request_irq

Few doubts regarding the mappings and child device handling.
Kindly, suggest me better methods.

Changes since v3:

1. Added clk_prepare_disable and regulator_disable calls in _remove()
2. Moved init_completion before irq_request
3. Added NULL pointer check for devm_request_and_ioremap() return value.
4. Use number of channels as per the ADC version 
5. Change the define ADC_V1_CHANNEL to ADC_CHANNEL
6. Update the Documentation to include EXYNOS5410 compatible

Doug, i've used
chan = iio_channel_get(dev_name(&pdev->dev), "adc3");
in ntc thermistor driver during probe and
iio_read_channel_raw(chan, &val);
for read.

But, then the drivers become kind of coupled. Right.

Lars, Is there an other way.

Thanks for the comments

 .../bindings/arm/samsung/exynos5-adc.txt   |   38 ++
 drivers/iio/adc/Kconfig|7 +
 drivers/iio/adc/Makefile   |1 +
 drivers/iio/adc/exynos5_adc.c  |  475 
 4 files changed, 521 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
 create mode 100644 drivers/iio/adc/exynos5_adc.c

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
new file mode 100644
index 000..0f281d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
@@ -0,0 +1,38 @@
+Samsung Exynos5 Analog to Digital Converter bindings
+
+Required properties:
+- compatible:  Must be "samsung,exynos5250-adc" for exynos5250 
controllers.
+   Must be "samsung,exynos5410-adc" for exynos5410 
controllers.
+- reg: Contains ADC register address range (base address and
+   length).
+- interrupts:  Contains the interrupt information for the timer. The
+   format is being dependent on which interrupt controller
+   the Samsung device uses.
+
+Note: child nodes can be added for auto probing from device tree.
+
+Example: adding device info in dtsi file
+
+adc@12D1 {
+   compatible = "samsung,exynos5250-adc";
+   reg = <0x12D1 0x100>;
+   interrupts = <0 106 0>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+};
+
+
+Example: Adding child nodes in dts file
+
+adc@12D1 {
+
+   /* NTC thermistor is a hwmon device */
+   ncp15wb473@0 {
+   compatible = "ntc,ncp15wb473";
+   reg = <0x0>;
+   pullup-uV = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   };
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index fe822a1..33ceabf 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -91,6 +91,13 @@ config AT91_ADC
help
  Say yes here to build support for Atmel AT91 ADC.
 
+config EXYNOS5_ADC
+   bool "Exynos5 ADC driver support"
+   help
+ Core support for the ADC block found in the Samsung EXYNOS5 series
+ of SoCs for drivers such as the touchscreen and hwmon to use to share
+ this resource.
+
 config LP8788_ADC
bool "LP8788 ADC driver"
depends on MFD_LP8788
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 2d5f100..5b4a4f6 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7793) += ad7793.o
 obj-$(CONFIG_AD7887) += ad7887.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
+obj-$(CONFIG_EXYNOS5_ADC) += exynos5_adc.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
diff --git a/drivers/iio/adc/exynos5_adc.c b/drivers/iio/adc/exynos5_adc.c
new file mode 100644
index 000..4963649
--- /dev/null
+++ b/drivers/iio/adc/exynos5_adc.c
@@ -0,0 +1,475 @@
+/*
+ *  exynos5_adc.c - Support for ADC in EXYNOS5 SoCs
+ *
+ *  8 ~ 10 channel, 10/12-bit ADC
+ *
+ *  Copyright (C) 2013 Naveen Krishna Chatradhi 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General 

Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-23 Thread Doug Anderson
Lars,

On Wed, Jan 23, 2013 at 4:52 AM, Lars-Peter Clausen  wrote:
>> Few doubts regarding the mappings and child device handling.
>> Kindly, suggest me better methods.
>
> The patch looks mostly good now. As for the mappings, the problem is that we
> currently do not have any device tree bindings for IIO. So a proper solution
> would be to add dt bindings for IIO.

Can you explain more how you think this would work?  It seems like
just having child nodes as platform drivers would be OK (to me) and
having them instantiated with of_platform_populate() seems reasonable.

...but the one thing that is missing is a way for children to get
access to the channel properly.

The children have access to the ADC "struct device" (it is their
parent device) and have a channel number (their "reg" field), but
there is no API call to map this to a "struct iio_channel".  Is that
all that's needed in this case?  ...or are you envisioning something
more?


Thanks!  :)


-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-23 Thread Lars-Peter Clausen
On 01/23/2013 05:58 AM, Naveen Krishna Chatradhi wrote:
> This patch add an ADC IP found on EXYNOS5 series socs from Samsung.
> Also adds the Documentation for device tree bindings.
> 
> Signed-off-by: Naveen Krishna Chatradhi 
> ---
> Changes since v1:
> 
> 1. Fixed comments from Lars
> 2. Added support for ADC on EXYNOS5410
> 
> Changes since v2:
> 
> 1. Changed the instance name for (struct iio_dev *) to indio_dev
> 2. Changed devm_request_irq to request_irq
> 
> Few doubts regarding the mappings and child device handling.
> Kindly, suggest me better methods.
> 

Hi,

The patch looks mostly good now. As for the mappings, the problem is that we
currently do not have any device tree bindings for IIO. So a proper solution
would be to add dt bindings for IIO.

[...]
> diff --git a/drivers/iio/adc/exynos5_adc.c b/drivers/iio/adc/exynos5_adc.c
> new file mode 100644
> index 000..8982675
> --- /dev/null
> +++ b/drivers/iio/adc/exynos5_adc.c
[...]
> +
> +/** ADC core in EXYNOS5410 has 10 channels,
> + * ADC core in EXYNOS5250 has 8 channels
> +*/

Minor nitpick, according to Documentation/Codingsytle multi-line comments
should look like this:

/*
 * Text
 * Text
 */

[...]
> +static int exynos5_adc_probe(struct platform_device *pdev)
> +{
> + struct exynos5_adc *info = NULL;
> + struct device_node *np = pdev->dev.of_node;
> + struct iio_dev *indio_dev = NULL;
> + struct resource *mem;
> + int ret = -ENODEV;
> + int irq;
> +
> + if (!np)
> + return ret;
> +
> + indio_dev = iio_device_alloc(sizeof(struct exynos5_adc));
> + if (!indio_dev) {
> + dev_err(&pdev->dev, "failed allocating iio device\n");
> + return -ENOMEM;
> + }
> +
> + info = iio_priv(indio_dev);
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + info->regs = devm_request_and_ioremap(&pdev->dev, mem);

While devm_request_and_ioremap takes care of the printing and error you
still need to check whether regs is non NULL.

> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "no irq resource?\n");
> + ret = irq;
> + goto err_iio;
> + }
> +
> + info->irq = irq;
> +
> + ret = request_irq(info->irq, exynos5_adc_isr,
> + 0, dev_name(&pdev->dev), info);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
> + info->irq);
> + goto err_iio;
> + }
> +
> + info->clk = devm_clk_get(&pdev->dev, "adc");
> + if (IS_ERR(info->clk)) {
> + dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
> + PTR_ERR(info->clk));
> + ret = PTR_ERR(info->clk);
> + goto err_irq;
> + }
> +
> + info->vdd = devm_regulator_get(&pdev->dev, "vdd");
> + if (IS_ERR(info->vdd)) {
> + dev_err(&pdev->dev, "failed getting regulator, err = %ld\n",
> + PTR_ERR(info->vdd));
> + ret = PTR_ERR(info->vdd);
> + goto err_irq;
> + }
> +
> + info->version = exynos5_adc_get_version(pdev);
> +
> + platform_set_drvdata(pdev, indio_dev);
> +
> + init_completion(&info->completion);

Since the completion is used in the IRQ handler it should be initialized
before the IRQ is requested.

> +
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->dev.of_node = pdev->dev.of_node;
> + indio_dev->info = &exynos5_adc_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = exynos5_adc_iio_channels;
> + indio_dev->num_channels = ARRAY_SIZE(exynos5_adc_iio_channels);

Shouldn't the number of channels depend on the ADC version? E.g. 8 for v1
and 10 for v2?

> +
> + info->map = exynos5_adc_iio_maps;
> +
> + ret = iio_map_array_register(indio_dev, info->map);
> + if (ret) {
> + dev_err(&indio_dev->dev, "failed mapping iio, err: %d\n", ret);
> + goto err_irq;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto err_map;
> +
> + ret = regulator_enable(info->vdd);
> + if (ret)
> + goto err_iio_dev;

You never disable the regulator again. I think you need to do that before it
is freed.

> +
> + clk_prepare_enable(info->clk);
> +
> + exynos5_adc_hw_init(info);
> +
> + ret = of_platform_populate(np, exynos5_adc_match, NULL, &pdev->dev);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed adding child nodes\n");
> + goto err_of_populate;
> + }
> +
> + return 0;
> +
> +err_of_populate:
> + device_for_each_child(&pdev->dev, NULL, exynos5_adc_remove_devices);
> +err_iio_dev:
> + iio_device_unregister(indio_dev);
> +err_map:
> + iio_map_a

[PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-22 Thread Naveen Krishna Chatradhi
This patch add an ADC IP found on EXYNOS5 series socs from Samsung.
Also adds the Documentation for device tree bindings.

Signed-off-by: Naveen Krishna Chatradhi 
---
Changes since v1:

1. Fixed comments from Lars
2. Added support for ADC on EXYNOS5410

Changes since v2:

1. Changed the instance name for (struct iio_dev *) to indio_dev
2. Changed devm_request_irq to request_irq

Few doubts regarding the mappings and child device handling.
Kindly, suggest me better methods.

 .../bindings/arm/samsung/exynos5-adc.txt   |   37 ++
 drivers/iio/adc/Kconfig|7 +
 drivers/iio/adc/Makefile   |1 +
 drivers/iio/adc/exynos5_adc.c  |  464 
 4 files changed, 509 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
 create mode 100644 drivers/iio/adc/exynos5_adc.c

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
new file mode 100644
index 000..9a5b515
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
@@ -0,0 +1,37 @@
+Samsung Exynos5 Analog to Digital Converter bindings
+
+Required properties:
+- compatible:  Must be "samsung,exynos5250-adc" for exynos5250 
controllers.
+- reg: Contains ADC register address range (base address and
+   length).
+- interrupts:  Contains the interrupt information for the timer. The
+   format is being dependent on which interrupt controller
+   the Samsung device uses.
+
+Note: child nodes can be added for auto probing from device tree.
+
+Example: adding device info in dtsi file
+
+adc@12D1 {
+   compatible = "samsung,exynos5250-adc";
+   reg = <0x12D1 0x100>;
+   interrupts = <0 106 0>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+};
+
+
+Example: Adding child nodes in dts file
+
+adc@12D1 {
+
+   /* NTC thermistor is a hwmon device */
+   ncp15wb473@0 {
+   compatible = "ntc,ncp15wb473";
+   reg = <0x0>;
+   pullup-uV = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   };
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index fe822a1..33ceabf 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -91,6 +91,13 @@ config AT91_ADC
help
  Say yes here to build support for Atmel AT91 ADC.
 
+config EXYNOS5_ADC
+   bool "Exynos5 ADC driver support"
+   help
+ Core support for the ADC block found in the Samsung EXYNOS5 series
+ of SoCs for drivers such as the touchscreen and hwmon to use to share
+ this resource.
+
 config LP8788_ADC
bool "LP8788 ADC driver"
depends on MFD_LP8788
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 2d5f100..5b4a4f6 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7793) += ad7793.o
 obj-$(CONFIG_AD7887) += ad7887.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
+obj-$(CONFIG_EXYNOS5_ADC) += exynos5_adc.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
diff --git a/drivers/iio/adc/exynos5_adc.c b/drivers/iio/adc/exynos5_adc.c
new file mode 100644
index 000..8982675
--- /dev/null
+++ b/drivers/iio/adc/exynos5_adc.c
@@ -0,0 +1,464 @@
+/*
+ *  exynos5_adc.c - Support for ADC in EXYNOS5 SoCs
+ *
+ *  8 ~ 10 channel, 10/12-bit ADC
+ *
+ *  Copyright (C) 2013 Naveen Krishna Chatradhi 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+enum adc_version {
+   ADC_V1,
+   ADC_V2
+};
+
+/* EXYNOS5250 ADC_V1 registers definitions */
+#define ADC_V1_CON(x)  ((x) + 0x00)
+#define ADC_V1_DLY(x)  ((x) + 0x08)
+#define ADC_V1_DATX(x) ((x) + 0x0C)
+#define ADC_V1_INTCLR(x)   ((x) + 0x18)
+#define ADC_V1_MUX(x)  ((x) + 0x1c)

[PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-22 Thread Naveen Krishna Chatradhi
This patch add an ADC IP found on EXYNOS5 series socs from Samsung.
Also adds the Documentation for device tree bindings.

Signed-off-by: Naveen Krishna Chatradhi 
---
Changes since v1:

1. Fixed comments from Lars
2. Added support for ADC on EXYNOS5410

Few doubts regarding the mappings.
Kindly, suggest me better methods.

 .../bindings/arm/samsung/exynos5-adc.txt   |   37 ++
 drivers/iio/adc/Kconfig|7 +
 drivers/iio/adc/Makefile   |1 +
 drivers/iio/adc/exynos5_adc.c  |  461 
 4 files changed, 506 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
 create mode 100644 drivers/iio/adc/exynos5_adc.c

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
new file mode 100644
index 000..9a5b515
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
@@ -0,0 +1,37 @@
+Samsung Exynos5 Analog to Digital Converter bindings
+
+Required properties:
+- compatible:  Must be "samsung,exynos5250-adc" for exynos5250 
controllers.
+- reg: Contains ADC register address range (base address and
+   length).
+- interrupts:  Contains the interrupt information for the timer. The
+   format is being dependent on which interrupt controller
+   the Samsung device uses.
+
+Note: child nodes can be added for auto probing from device tree.
+
+Example: adding device info in dtsi file
+
+adc@12D1 {
+   compatible = "samsung,exynos5250-adc";
+   reg = <0x12D1 0x100>;
+   interrupts = <0 106 0>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+};
+
+
+Example: Adding child nodes in dts file
+
+adc@12D1 {
+
+   /* NTC thermistor is a hwmon device */
+   ncp15wb473@0 {
+   compatible = "ntc,ncp15wb473";
+   reg = <0x0>;
+   pullup-uV = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   };
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index fe822a1..33ceabf 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -91,6 +91,13 @@ config AT91_ADC
help
  Say yes here to build support for Atmel AT91 ADC.
 
+config EXYNOS5_ADC
+   bool "Exynos5 ADC driver support"
+   help
+ Core support for the ADC block found in the Samsung EXYNOS5 series
+ of SoCs for drivers such as the touchscreen and hwmon to use to share
+ this resource.
+
 config LP8788_ADC
bool "LP8788 ADC driver"
depends on MFD_LP8788
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 2d5f100..5b4a4f6 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7793) += ad7793.o
 obj-$(CONFIG_AD7887) += ad7887.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
+obj-$(CONFIG_EXYNOS5_ADC) += exynos5_adc.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
diff --git a/drivers/iio/adc/exynos5_adc.c b/drivers/iio/adc/exynos5_adc.c
new file mode 100644
index 000..62643f5
--- /dev/null
+++ b/drivers/iio/adc/exynos5_adc.c
@@ -0,0 +1,461 @@
+/*
+ *  exynos5_adc.c - Support for ADC in EXYNOS5 SoCs
+ *
+ *  8 ~ 10 channel, 10/12-bit ADC
+ *
+ *  Copyright (C) 2013 Naveen Krishna Chatradhi 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+enum adc_version {
+   ADC_V1,
+   ADC_V2
+};
+
+/* EXYNOS5250 ADC_V1 registers definitions */
+#define ADC_V1_CON(x)  ((x) + 0x00)
+#define ADC_V1_DLY(x)  ((x) + 0x08)
+#define ADC_V1_DATX(x) ((x) + 0x0C)
+#define ADC_V1_INTCLR(x)   ((x) + 0x18)
+#define ADC_V1_MUX(x)  ((x) + 0x1c)
+
+/* EXYNOS5410 ADC_V2 registers definitions */
+#define ADC_V2_CON1(x) ((x) + 0x00)
+#define ADC_V2_CON2(x) ((x) + 0x04)
+#define ADC_V2

Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-22 Thread Naveen Krishna Ch
Hello All,

On 22 January 2013 15:14, Lars-Peter Clausen  wrote:
>
> Hi,
>
> On 01/21/2013 02:37 PM, Naveen Krishna Chatradhi wrote:
> > This patch add an ADC IP found on EXYNOS5 series socs from Samsung.
> > Also adds the Documentation for device tree bindings.
> >[...]
> > diff --git a/drivers/iio/adc/exynos5_adc.c
> > b/drivers/iio/adc/exynos5_adc.c
> > new file mode 100644
> > index 000..cd33ea2
> > --- /dev/null
> > +++ b/drivers/iio/adc/exynos5_adc.c
> > @@ -0,0 +1,412 @@
> > +/*
> > + *  exynos5_adc.c - Support for ADC in EXYNOS5 SoCs
> > + *
> > + *  8-channel, 10/12-bit ADC
> > + *
> > + *  Copyright (C) 2013 Naveen Krishna Chatradhi 
> > + *
> > + *  This program is free software; you can redistribute it and/or
> > modify
> > + *  it under the terms of the GNU General Public License as published
> > by
> > + *  the Free Software Foundation; either version 2 of the License, or
> > + *  (at your option) any later version.
> > + *
> > + *  This program is distributed in the hope that it will be useful,
> > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + *  GNU General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public License
> > + *  along with this program; if not, write to the Free Software
> > + *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* Samsung ADC registers definitions */
> > +#define EXYNOS_ADC_CON(x)((x) + 0x00)
> > +#define EXYNOS_ADC_DLY(x)((x) + 0x08)
> > +#define EXYNOS_ADC_DATX(x)   ((x) + 0x0C)
> > +#define EXYNOS_ADC_INTCLR(x) ((x) + 0x18)
> > +#define EXYNOS_ADC_MUX(x)((x) + 0x1c)
> > +
> > +/* Bit definitions for EXYNOS_ADC_MUX: */
> > +#define ADC_RES  (1u << 16)
> > +#define ADC_ECFLG(1u << 15)
> > +#define ADC_PRSCEN   (1u << 14)
> > +#define ADC_PRSCLV(x)(((x) & 0xFF) << 6)
> > +#define ADC_PRSCVLMASK   (0xFF << 6)
> > +#define ADC_STANDBY  (1u << 2)
> > +#define ADC_READ_START   (1u << 1)
> > +#define ADC_EN_START (1u << 0)
>
> You are a bit inconsistent with your prefixes, sometimes you use
> EXYNOS_ADC,
> sometimes just ADC, it would be better to use the same prefix all the
> time.
Will correct that.
>
> > +
> > +#define ADC_DATX_MASK0xFFF
> > +
> > +struct exynos5_adc {
> > + void __iomem*regs;
> > + struct clk  *clk;
> > + unsigned intirq;
> > + struct regulator*vdd;
> > +
> > + struct completion   completion;
> > +
> > + struct iio_map  *map;
> > + u32 value;
> > + u32 prescale;
> > +};
> > +
> > +static const struct of_device_id exynos5_adc_match[] = {
> > + { .compatible = "samsung,exynos5250-adc" },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, exynos5_adc_match);
> > +
> > +/* default maps used by iio consumer (ex: ntc-thermistor driver) */
> > +static struct iio_map exynos5_adc_iio_maps[] = {
> > + {
> > + .consumer_dev_name = "0.ncp15wb473",
> > + .consumer_channel = "adc_user3",
> > + .adc_channel_label = "adc3",
> > + },
> > + {},
> > +};
>
> Hm... this should not be in the driver itself.
I've taken the example of "drivers/iio/adc/lp8788_adc.c"
I think this one is a tightly coupled driver.

Kindly, suggest me a better example.

>
> > +
> > +static int exynos5_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val,
> > + int *val2,
> > + long mask)
> > +{
> > + struct exynos5_adc *info = iio_priv(indio_dev);
> > + u32 con;
> > +
> > + if (mask == IIO_CHAN_INFO_RAW) {
> > + mutex_lock(&indio_dev->mlock);
> > +
> > + /* Select the channel to be used */
> > + writel(chan->address, EXYNOS_ADC_MUX(info->regs));
> > + /* Trigger conversion */
> > + con = readl(EXYNOS_ADC_CON(info->regs));
> > + writel(con | ADC_EN_START, EXYNOS_ADC_CON(info->regs));
> > +
> > + wait_for_completion(&info->completion);
> > + *val = info->value;
> > +
> > + mutex_unlock(&indio_dev->mlock);
> > +
> > + return IIO_VAL_INT;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static irqreturn_t exynos5_adc_isr(int irq, void *dev_id)
> > +{
> > + struct exynos5_adc *info = (struct exynos5_adc *)dev_id;
> > +
> > + /* Read value and clear irq */
> > + info->value = readl(EXYNOS_ADC_DATX(info->regs)) &
> > +

Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-22 Thread Lars-Peter Clausen
Hi,

On 01/21/2013 02:37 PM, Naveen Krishna Chatradhi wrote:
> This patch add an ADC IP found on EXYNOS5 series socs from Samsung.
> Also adds the Documentation for device tree bindings.
>[...]
> diff --git a/drivers/iio/adc/exynos5_adc.c b/drivers/iio/adc/exynos5_adc.c
> new file mode 100644
> index 000..cd33ea2
> --- /dev/null
> +++ b/drivers/iio/adc/exynos5_adc.c
> @@ -0,0 +1,412 @@
> +/*
> + *  exynos5_adc.c - Support for ADC in EXYNOS5 SoCs
> + *
> + *  8-channel, 10/12-bit ADC
> + *
> + *  Copyright (C) 2013 Naveen Krishna Chatradhi 
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +/* Samsung ADC registers definitions */
> +#define EXYNOS_ADC_CON(x)((x) + 0x00)
> +#define EXYNOS_ADC_DLY(x)((x) + 0x08)
> +#define EXYNOS_ADC_DATX(x)   ((x) + 0x0C)
> +#define EXYNOS_ADC_INTCLR(x) ((x) + 0x18)
> +#define EXYNOS_ADC_MUX(x)((x) + 0x1c)
> +
> +/* Bit definitions for EXYNOS_ADC_MUX: */
> +#define ADC_RES  (1u << 16)
> +#define ADC_ECFLG(1u << 15)
> +#define ADC_PRSCEN   (1u << 14)
> +#define ADC_PRSCLV(x)(((x) & 0xFF) << 6)
> +#define ADC_PRSCVLMASK   (0xFF << 6)
> +#define ADC_STANDBY  (1u << 2)
> +#define ADC_READ_START   (1u << 1)
> +#define ADC_EN_START (1u << 0)

You are a bit inconsistent with your prefixes, sometimes you use EXYNOS_ADC,
sometimes just ADC, it would be better to use the same prefix all the time.

> +
> +#define ADC_DATX_MASK0xFFF
> +
> +struct exynos5_adc {
> + void __iomem*regs;
> + struct clk  *clk;
> + unsigned intirq;
> + struct regulator*vdd;
> +
> + struct completion   completion;
> +
> + struct iio_map  *map;
> + u32 value;
> + u32 prescale;
> +};
> +
> +static const struct of_device_id exynos5_adc_match[] = {
> + { .compatible = "samsung,exynos5250-adc" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, exynos5_adc_match);
> +
> +/* default maps used by iio consumer (ex: ntc-thermistor driver) */
> +static struct iio_map exynos5_adc_iio_maps[] = {
> + {
> + .consumer_dev_name = "0.ncp15wb473",
> + .consumer_channel = "adc_user3",
> + .adc_channel_label = "adc3",
> + },
> + {},
> +};

Hm... this should not be in the driver itself.

> +
> +static int exynos5_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val,
> + int *val2,
> + long mask)
> +{
> + struct exynos5_adc *info = iio_priv(indio_dev);
> + u32 con;
> +
> + if (mask == IIO_CHAN_INFO_RAW) {
> + mutex_lock(&indio_dev->mlock);
> +
> + /* Select the channel to be used */
> + writel(chan->address, EXYNOS_ADC_MUX(info->regs));
> + /* Trigger conversion */
> + con = readl(EXYNOS_ADC_CON(info->regs));
> + writel(con | ADC_EN_START, EXYNOS_ADC_CON(info->regs));
> +
> + wait_for_completion(&info->completion);
> + *val = info->value;
> +
> + mutex_unlock(&indio_dev->mlock);
> +
> + return IIO_VAL_INT;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static irqreturn_t exynos5_adc_isr(int irq, void *dev_id)
> +{
> + struct exynos5_adc *info = (struct exynos5_adc *)dev_id;
> +
> + /* Read value and clear irq */
> + info->value = readl(EXYNOS_ADC_DATX(info->regs)) &
> + ADC_DATX_MASK;
> + writel(0, EXYNOS_ADC_INTCLR(info->regs));
> +
> + complete(&info->completion);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int exynos5_adc_reg_access(struct iio_dev *indio_dev,
> +   unsigned reg, unsigned writeval,
> +   unsigned *readval)
> +{
> + struct exynos5_adc *info = iio_priv(indio_dev);
> + u32 ret;
> +
> + mutex_lock(&indio_dev->mlock);

Do we really need to take the lock here?

> +
> + if (readval != NU

[PATCH] iio: adc: add exynos5 adc driver under iio framwork

2013-01-21 Thread Naveen Krishna Chatradhi
This patch add an ADC IP found on EXYNOS5 series socs from Samsung.
Also adds the Documentation for device tree bindings.

Signed-off-by: Naveen Krishna Chatradhi 
---
 .../bindings/arm/samsung/exynos5-adc.txt   |   20 +
 drivers/iio/adc/Kconfig|7 +
 drivers/iio/adc/Makefile   |1 +
 drivers/iio/adc/exynos5_adc.c  |  412 
 4 files changed, 440 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
 create mode 100644 drivers/iio/adc/exynos5_adc.c

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
new file mode 100644
index 000..069639e
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos5-adc.txt
@@ -0,0 +1,20 @@
+Samsung Exynos5 Analog to Digital Converter bindings
+
+Required properties:
+- compatible:  Must be "samsung,exynos5250-adc" for exynos5250 
controllers.
+- reg: Contains ADC register address range (base address and
+   length).
+- interrupts:  Contains the interrupt information for the timer. The
+   format is being dependent on which interrupt controller
+   the Samsung device uses.
+
+Example:
+
+adc@12D1 {
+   compatible = "samsung,exynos5250-adc";
+   reg = <0x12D1 0x100>;
+   interrupts = <0 106 0>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index fe822a1..33ceabf 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -91,6 +91,13 @@ config AT91_ADC
help
  Say yes here to build support for Atmel AT91 ADC.
 
+config EXYNOS5_ADC
+   bool "Exynos5 ADC driver support"
+   help
+ Core support for the ADC block found in the Samsung EXYNOS5 series
+ of SoCs for drivers such as the touchscreen and hwmon to use to share
+ this resource.
+
 config LP8788_ADC
bool "LP8788 ADC driver"
depends on MFD_LP8788
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 2d5f100..5b4a4f6 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
 obj-$(CONFIG_AD7793) += ad7793.o
 obj-$(CONFIG_AD7887) += ad7887.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
+obj-$(CONFIG_EXYNOS5_ADC) += exynos5_adc.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
diff --git a/drivers/iio/adc/exynos5_adc.c b/drivers/iio/adc/exynos5_adc.c
new file mode 100644
index 000..cd33ea2
--- /dev/null
+++ b/drivers/iio/adc/exynos5_adc.c
@@ -0,0 +1,412 @@
+/*
+ *  exynos5_adc.c - Support for ADC in EXYNOS5 SoCs
+ *
+ *  8-channel, 10/12-bit ADC
+ *
+ *  Copyright (C) 2013 Naveen Krishna Chatradhi 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/* Samsung ADC registers definitions */
+#define EXYNOS_ADC_CON(x)  ((x) + 0x00)
+#define EXYNOS_ADC_DLY(x)  ((x) + 0x08)
+#define EXYNOS_ADC_DATX(x) ((x) + 0x0C)
+#define EXYNOS_ADC_INTCLR(x)   ((x) + 0x18)
+#define EXYNOS_ADC_MUX(x)  ((x) + 0x1c)
+
+/* Bit definitions for EXYNOS_ADC_MUX: */
+#define ADC_RES(1u << 16)
+#define ADC_ECFLG  (1u << 15)
+#define ADC_PRSCEN (1u << 14)
+#define ADC_PRSCLV(x)  (((x) & 0xFF) << 6)
+#define ADC_PRSCVLMASK (0xFF << 6)
+#define ADC_STANDBY(1u << 2)
+#define ADC_READ_START (1u << 1)
+#define ADC_EN_START   (1u << 0)
+
+#define ADC_DATX_MASK  0xFFF
+
+struct exynos5_adc {
+   void __iomem*regs;
+   struct clk  *clk;
+   unsigned intirq;
+   struct regulator*vdd;
+
+   struct completion   completion;
+
+   struct iio_map  *map;
+   u32 value;
+   u32 prescale;
+};
+
+static const struct of_device_id exynos5_adc_match[] = {
+   { .compatible = "samsung,exynos525