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> >>> >>> 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
