Hi Alex, Please see my response inline. > -----Original Message----- > From: Alex Riesen [mailto:raa.lkml at gmail.com] > Sent: Friday, August 14, 2009 12:44 AM > To: Agarwal, Ramesh > Cc: lm-sensors at lm-sensors.org; Mark Brown > Subject: Re: Intel Moorestown Analog Accelerometer driver > > Ramesh Agarwal says: > > +The accelerometer operates in the polling mode. This is to say that > sampling > > +by the driver will be done only if a user application has requested for > the > > +values. The driver does not register as an input device, and hence > input > > +calibration and fixing the sampling rate is left to the user level > > +framework/application. The sysfs interface only provides the decimal > values > > +of the voltage read from the ADC where each bit is equivalent to 2mV. > ... > > +/* PMIC ADC INTERRUPT REGISTERS */ > > +#define PMIC_ADC_ACC_REG_ADCINT 0x5F /*ADC interrupt register > */ > > Does the accelerometer itself actually provide an interrupt for the > data? Can the interrupt be used? > > The "ADC interrupt register" seem to hint at that. > > Forcing people to poll for interactive data (and that seems to be a > very common use for accelerometers these days) is not very nice. [Agarwal, Ramesh] The accelerometer unfortunately does not have any interrupt. It just gives the voltage for the current value of the g on the three co-ordinates. The Interrupt being referred to is for the ADC. Once the ADC is started it will sample all the channels (including others which are used by other analog devices) in a round robin fashion. The interrupt is raised when one round of sampling completes. > > > +static unsigned int analog_accel_read(int offset) > > +{ > > + unsigned int ret_val; > > + struct mrst_pmic_reg_data ipc_data; > > + > > + ipc_data.ioc = 0; /* No need to generate MSI */ > > + ipc_data.num_entries = 2; > > + ipc_data.pmic_reg_data[0].register_address = > > + PMIC_ADC_REG_HIGH(mrst_analog_reg_idx, > > + offset); /* Higher 8 bits */ > > + ipc_data.pmic_reg_data[1].register_address = > > + PMIC_ADC_REG_LOW(mrst_analog_reg_idx, > > + offset); /* Lower 3 bits */ > > + if (mrst_pmic_ioread(&ipc_data) != 0) { > > + printk(KERN_ERR > > + "mrst_analog_accel:PMIC reg read using IPC failed\n"); > > + return -1; > > You return -1 in a function returning unsigned? > And you don't handle the -1 anyway, so maybe a more useful value can > be returned (like one you'd return when there is no acceleration). > Or you can handle the error more sensibly, and return error (if it's > possible) from sysfs' _show(). > [Agarwal, Ramesh] Sending 0xffffffff was kind of my way of indicating an error because the values coming out of the ADC are just 10 bits, so anything larger than that would indicate that there was an error reading the values.
