linux-kernel - Re: [PATCH 5/5] Input: add CMR3000 gyrsocope driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 18 Oct 2011 14:07:32 +0200
From:	Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
To:	Jonathan Cameron <jic23@....ac.uk>
Cc:	dmitry.torokhov@...il.com, sameo@...ux.intel.com,
	peter.ujfalusi@...com, aghayal@...eaurora.org, david@...deman.nu,
	Shubhrajyoti@...com, saaguirre@...com, hemanthv@...com,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/5] Input: add CMR3000 gyrsocope driver

Hello Jonathan

On Tue, Oct 18, 2011 at 10:54, Jonathan Cameron <jic23@....ac.uk> wrote:
> On 10/18/11 09:28, Ricardo Ribalda Delgado wrote:

> Quick review done below.  Quite a bit of overlap with earlier reviews.
> Sorry I couldn't take a closer look.

I am fixing also previous errors

> accl?  I thought this was a gyro so this seems odd.
copy paste :), fixing it

> I'd much rather see these done explicity in the code.
> This sort of macro just obscures the works to save a few lines
> of code.
>> +#define CMR3000_READ(data, reg, msg) \
>> +     (data->bus_ops->read(data->dev, reg, msg))
>> +#define CMR3000_SET(data, reg, val, msg) \
>> +     ((data)->bus_ops->write(data->dev, reg, val, msg))
>> +

Fixing it


>> +void cmr3000_exit(struct cmr3000_accl_data *data)
>> +{
>> +     free_irq(data->irq, data);
>> +     input_unregister_device(data->input_dev);
> cleaner to unwind in reverse order of setup unless you have
> a good reason and that should be commented.

Fixing it, and also fixing it on the cma3000

> Match comment to name fo structure.
>> + * struct cmr3000_i2c_platform_data - CMR3000 Platform data
>> + * @fuzz_x: Noise on X Axis
>> + * @fuzz_y: Noise on Y Axis
>> + * @fuzz_z: Noise on Z Axis
>> + * @mode: Operating mode
>> + * @irq_level: Irq level
>> + */
>> +struct cmr3000_platform_data {
>> +     int fuzz_x;
>> +     int fuzz_y;
>> +     int fuzz_z;
>> +     uint8_t irq_level;
>> +     uint8_t mode;
>> +     unsigned long irqflags;
>> +};

Fixing it, and also fixing it on the cma3000


Thanks!



-- 
Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/