On 02/15/2013 01:35 PM, Naveen Krishna Ch wrote: > On 15 February 2013 18:56, Lars-Peter Clausen <lars@xxxxxxxxxx> wrote: >> On 02/15/2013 02:17 PM, Naveen Krishna Ch wrote: >>> On 15 February 2013 18:43, Lars-Peter Clausen <lars@xxxxxxxxxx> wrote: >>>> On 02/15/2013 07:56 AM, Naveen Krishna Chatradhi wrote: >>>>> This patch adds New driver to support: >>>>> 1. Supports ADC IF found on EXYNOS4412/EXYNOS5250 >>>>> and future SoCs from Samsung >>>>> 2. Add ADC driver under iio/adc framework >>>>> 3. Also adds the Documentation for device tree bindings >>>>> >>>>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@xxxxxxxxxxx> >>>> >>>> Looks good. >>>> >>>> Reviewed-by: Lars-Peter Clausen <lars@xxxxxxxxxx> Applied to togreg branch of iio.git Thanks, >>>> >>>> One minor thing though, there are a couple of places where you break a line >>>> into multiple lines, even though the line fits easily inside the 80 chars >>>> per line limit. In my opinion this doesn't help the legibility of the code. >>>> E.g.: >>>> >>>> + info->value = readl(ADC_V1_DATX(info->regs)) & >>>> + ADC_DATX_MASK; >>>> >>>> There is no need to respin the patch just for this, but if you happen to >>>> make another version of the patch, that's something to consider. >>>> >>>>> --- >>>>> 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 >>>>> >>>>> Changes since v5: >>>>> >>>>> 1. Fixed comments from Olof (ADC hardware version handling) >>>>> 2. Rebased on top of comming OF framework for IIO by "Guenter Roeck". >>>>> >>>>> Changes since v6: >>>>> >>>>> 1. Addressed comments from Lars-Peter Clausen >>>> >>>> >>>> btw. these kind of change logs are not really helpful, it would be better to >>>> list the actual changes made. >>> Hello Lars, >>> >>> No other changes from my side. But, I can send another version. >>> Do you want me to list the latest change alone instead of the whole >>> change list ? >> >> Hi, >> >> No need to resend the patch, this is just something to consider for the >> future. A changelog entry which reads like "Addressed Jon Does comments" is >> not really useful since most people will probably not know or not longer >> remember all the details of those comments, instead a nice list of all the >> changes which have been made is much better. E.g.: >> >> Changes since v6: >> * Fixed debugfs_read_reg >> * Introduced timeout when waiting for the data ready IRQ >> * Slightly reformatted exynos_read_raw for better legibility >> >> - Lars > > Thanks for your comments and valuable time. > Sure Lars, Will do it. >> >> > > > > -- > Shine bright, > (: Nav :) > -- > 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 > -- 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
