Re: [PATCH 1/2 v2] mfd: twl4030: Driver for twl4030 madc module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 28, 2011 at 6:30 AM, Samuel Ortiz <sameo@xxxxxxxxxxxxxxx> wrote:
> Hi Keerthy,
>
> On Thu, Feb 24, 2011 at 08:48:33PM +0530, Keerthy wrote:
>> Introducing a driver for MADC on TWL4030 powerIC. MADC stands for monitoring
>> ADC. This driver monitors the real time conversion of analog signals like
>> battery temperature, battery cuurent etc.
> Looks mostly good to me, one minor comment though:
>
>> +static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
>> +                                   u8 reg_base, u16 channels, int *buf)
>> +{
>> +     int count = 0, count_req = 0;
>> +     u8 reg, i;
>> +
>> +     for (i = 0; i < TWL4030_MADC_MAX_CHANNELS; i++) {
>> +             if (channels & (1 << i)) {
> You may want to use for_each_set_bit() here.
>

Ok.
>
>> +                     reg = reg_base + 2 * i;
>> +                     buf[i] = twl4030_madc_channel_raw_read(madc, reg);
>> +                     if (buf[i] < 0) {
>> +                             dev_err(madc->dev,
>> +                             "Unable to read register 0x%X\n", reg);
>> +                             count_req++;
> For a clearer code, just call continue here, and the following blck will look
> nicer.
>

I will call continue.
>> +static irqreturn_t twl4030_madc_threaded_irq_handler(int irq, void *_madc)
>> +{
>> +     struct twl4030_madc_data *madc = _madc;
>> +     const struct twl4030_madc_conversion_method *method;
>> +     u8 isr_val, imr_val;
>> +     int i, len, ret;
>> +     struct twl4030_madc_request *r;
>
> Don't you want to take the madc lock around here ?

Ok.
>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/
>



-- 
Regards and Thanks,
Keerthy
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux