Messages in this thread | | | Subject | Re: [PATCH v2 1/2] iio: gyro: Add driver support for ADXRS290 | From | Nishant Malpani <> | Date | Wed, 22 Jul 2020 19:59:11 +0530 |
| |
On 22/07/20 3:53 pm, Andy Shevchenko wrote: > On Wed, Jul 22, 2020 at 12:40 PM Nishant Malpani > <nish.malpani25@gmail.com> wrote: >> On 22/07/20 3:08 am, Andy Shevchenko wrote: >>> On Tue, Jul 21, 2020 at 11:35 PM Nishant Malpani >>> <nish.malpani25@gmail.com> wrote: >>>> On 22/07/20 1:16 am, Andy Shevchenko wrote: > > ... > >>> Can't you declare table as const int? >>> >> I'm not sure I understand you completely here; do you mean const int *? >> So, an array of alternate integer and fractional parts? I suppose that's >> possible but we'd be introducing unwanted complexity I feel - for >> example, currently the index of the 3db frequency in the table is used >> to directly map & set bits in the filter register corresponding to that >> frequency but with the approach you share, we'd have to apply a >> transformation (div by 2) to set the same bits in the filter register. >> Do you think the added complexity justifies the removal of the casting? > > It was a question. If you think it is too much, don't change :-) > > ... > >>>>>> + /* max transition time to measurement mode */ >>>>>> + msleep_interruptible(ADXRS290_MAX_TRANSITION_TIME_MS); >>>>> >>>>> I'm not sure what the point of interruptible variant here? >>>>> >>>> I referred Documentation/timers/timers-howto.rst for this. >>>> My reasoning was shaped to use the interruptible variant because the >>>> transition settles in a time *less than* 100ms and since 100ms is quite >>>> a huge time to sleep, it should be interrupted in case a signal arrives. >>> >>> This is probe of the device, >>> What are the expectations here? >>> >> I fail to understand why this can't be used in the probe() but perhaps >> in a routine to standby/resume. Could you please elaborate? > > I didn't say it can not be used, what I'm asking is what are the > expectations of the interruptible part here. > In other words what is the benefit that makes you choose this over > plain msleep(). > Oh, sorry for I misunderstood.
I cannot think of anything more to add to the reasoning that I explained earlier; in that case, I'll fall back to msleep() (in v3) unless someone else comes with a strong point in msleep_interruptible()'s favor.
With regards, Nishant Malpani
| |