linux-kernel - Re: [PATCH v2 1/2] iio: gyro: Add driver support for ADXRS290

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:   Wed, 22 Jul 2020 13:23:22 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Nishant Malpani <nish.malpani25@...il.com>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        "Bogdan, Dragos" <dragos.bogdan@...log.com>,
        darius.berghe@...log.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-iio <linux-iio@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] iio: gyro: Add driver support for ADXRS290

On Wed, Jul 22, 2020 at 12:40 PM Nishant Malpani
<nish.malpani25@...il.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@...il.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().


-- 
With Best Regards,
Andy Shevchenko