Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200721181926.27046-1-nish.malpani25@gmail.com>
 <CAHp75Vdr+Uo2uw3mzYP+LMRgp-eyi+YjG=O+wGVqyYx-+MRCaw@mail.gmail.com>
 <7ba8469a-dd8c-1686-6d26-e2a4cbfedce9@gmail.com> <CAHp75VdYVC9n7-2MH62J46N0p+sNSE9QVwonor5QfdnvL4hoLg@mail.gmail.com>
 <5cb55101-af5c-b6a2-d770-9717f8a463cc@gmail.com>
In-Reply-To: <5cb55101-af5c-b6a2-d770-9717f8a463cc@gmail.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Wed, 22 Jul 2020 13:23:22 +0300
Message-ID: <CAHp75Vc5e0HfVe04yzyfGC_qqhcPNnJOHXcADLfz+RKMuFBbcA@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] iio: gyro: Add driver support for ADXRS290
To:     Nishant Malpani <nish.malpani25@gmail.com>
Cc:     Jonathan Cameron <jic23@kernel.org>,
        "Bogdan, Dragos" <dragos.bogdan@analog.com>,
        darius.berghe@analog.com,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-iio <linux-iio@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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().


-- 
With Best Regards,
Andy Shevchenko