Re: [IR-RFC PATCH v4 2/6] Core IR module

[Jon Smirl <jonsmirl@gmail.com>]

On Sun, Nov 29, 2009 at 12:17 PM, Greg KH <greg@kroah.com> wrote:
>> +static ssize_t ir_raw_show(struct device *dev,
>> +                              struct device_attribute *attr, char *buf)
>> +{
>> +     struct input_dev *input_dev = to_input_dev(dev);
>> +     unsigned int i, count = 0;
>> +
>> +     for (i = input_dev->ir->raw.tail; i != input_dev->ir->raw.head; ) {
>> +
>> +             count += snprintf(&buf[count], PAGE_SIZE - 1, "%i\n", input_dev->ir->raw.buffer[i++]);
>> +             if (i > ARRAY_SIZE(input_dev->ir->raw.buffer))
>> +                     i = 0;
>> +             if (count >= PAGE_SIZE - 1) {
>> +                     input_dev->ir->raw.tail = i;
>> +                     return PAGE_SIZE - 1;
>> +             }
>> +     }
>> +     input_dev->ir->raw.tail = i;
>> +     return count;
>> +}
>
> This looks like it violates the "one value per sysfs file" rule that we
> have.  What exactly are you outputting here?  It does not look like this
> belongs in sysfs at all.

It should be on a debug switch or maybe a debug device. If the rest of
the system is working right you shouldn't need this data.

>
>> +static ssize_t ir_raw_store(struct device *dev,
>> +                               struct device_attribute *attr,
>> +                               const char *buf,
>> +                               size_t count)
>> +{
>> +     struct ir_device *ir = to_input_dev(dev)->ir;
>> +     long delta;
>> +     int i = count;
>> +     int first = 0;
>> +
>> +     if (!ir->xmit)
>> +             return count;
>> +     ir->send.count = 0;
>> +
>> +     while (i > 0) {
>> +             i -= strict_strtoul(&buf[i], i, &delta);
>> +             while ((buf[i] != '\n') && (i > 0))
>> +                     i--;
>> +             i--;
>> +             /* skip leading zeros */
>> +             if ((delta > 0) && !first)
>> +                     continue;
>> +
>> +             ir->send.buffer[ir->send.count++] = abs(delta);
>> +     }
>> +
>> +     ir->xmit(ir->private, ir->send.buffer, ir->send.count, ir->raw.carrier, ir->raw.xmitter);
>> +
>> +     return count;
>> +}
>
> What type of data are you expecting here?  More than one value?
>
> thanks,
>
> greg k-h
>



-- 
Jon Smirl
jonsmirl@gmail.com