Re: [PATCH v2 01/01] media: gscaler: Add new driver for generic scaler

[Sylwester Nawrocki <sylvester.nawrocki@gmail.com>]

On 07/12/2012 12:39 AM, Sylwester Nawrocki wrote:
>> +int gsc_hw_get_input_buf_mask_status(struct gsc_dev *dev)
>> +{
>> +	u32 cfg, status, bits = 0;
>> +
>> +	cfg = readl(dev->regs + GSC_IN_BASE_ADDR_Y_MASK);

	return hweight32(cfg & GSC_IN_BASE_ADDR_MASK);

>> +	status = cfg&   GSC_IN_BASE_ADDR_MASK;
>> +	while (status) {
>> +		status = status&   (status - 1);
>> +		bits++;
>> +	}
>> +	return bits;
>> +}
[...]
>> +
>> +int gsc_hw_get_nr_unmask_bits(struct gsc_dev *dev)
>> +{
>> +	u32 bits = 0;
>> +	u32 mask_bits = readl(dev->regs + GSC_OUT_BASE_ADDR_Y_MASK);
>
> Care to add an empty line? It might also look better to order
> declarations in decending line order.
>
>> +	mask_bits&= GSC_OUT_BASE_ADDR_MASK;
>> +
>> +	while (mask_bits) {
>> +		mask_bits = mask_bits&   (mask_bits - 1);
>> +		bits++;
>> +	}

It seems you are computing a Hamming weight here, then it could
be simplified to a built-in function:

	bits = hweight32(mask_bits);

But it's not quite clear to me, why you're subtracting it
from 16 ?

>> +	bits = 16 - bits;
>
> return 16 - bits;
>
>> +
>> +	return bits;
>> +}