Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Apr 2023 14:20:15 +0200
From:   Andi Shyti <andi.shyti@kernel.org>
To:     Matti Vaittinen <mazziesaccount@gmail.com>
Cc:     Andi Shyti <andi.shyti@kernel.org>,
        Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
        Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Zhigang Shi <Zhigang.Shi@liteon.com>,
        Shreeya Patel <shreeya.patel@collabora.com>,
        Paul Gazzillo <paul@pgazz.com>,
        Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/5] iio: light: ROHM BU27008 color sensor
Message-ID: <20230426122015.cilet7diaxapgbim@intel.intel>
References: <cover.1682340947.git.mazziesaccount@gmail.com>
 <d51d5e2b3eff65fd86aeb47840db9cd413d96668.1682340947.git.mazziesaccount@gmail.com>
 <20230425164522.sljcniui5ox5yx3l@intel.intel>
 <102a1605-d6dc-80c7-2075-212569c97042@gmail.com>
 <20230426101200.7czyp6nlg44tweyb@intel.intel>
 <d1121acc-6300-011f-caa2-0ddc01c46e91@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d1121acc-6300-011f-caa2-0ddc01c46e91@gmail.com>
Precedence: bulk

Hi Matti,

> > > > > +static int bu27008_chip_init(struct bu27008_data *data)
> > > > > +{
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = regmap_update_bits(data->regmap, BU27008_REG_SYSTEM_CONTROL,
> > > > > +			   BU27008_MASK_SW_RESET, BU27008_MASK_SW_RESET);
> > > > > +	if (ret)
> > > > > +		return dev_err_probe(data->dev, ret, "Sensor reset failed\n");
> > > > > +
> > > > > +	/*
> > > > > +	 * The data-sheet does not tell how long performing the IC reset takes.
> > > > > +	 * However, the data-sheet says the minimum time it takes the IC to be
> > > > > +	 * able to take inputs after power is applied, is 100 uS. I'd assume
> > > > > +	 * > 1 mS is enough.
> > > > > +	 */
> > > > > +	msleep(1);
> > > > 
> > > > please use usleep_range().
> > > 
> > > I prefer to not require setting up hrtimers as we have no real requirements
> > > for the duration of this sleep. I know the msleep() is likely to exceed the
> > > 1 mS, potentially a lot if there is things to do - but we don't really care
> > > at this point. The main thing is to give the HW time to reset while allowing
> > > other things to be scheduled.
> > 
> > For the reason above, msleep(1) is quite a meaningless
> > instruction. If you need to wait around 1ms, then usleep_range is
> > the function to be used.
> > 
> > Refer, also, to the Documentation/timers/timers-howto.rst
> 
> I know the msleep() may sleep much longer. It still is not meaningless. Here
> we use the msleep() in a purpose:
> 
> "Sleep at least 1 mS, but actually I don't care if that is 20 mS or more -
> as long as you allow scheduling other things with as little overhead as
> possible".
> 
> For that purpose msleep() works just perfectly :)
> 
> I actually had a comment clarifying this in previous IIO driver I wrote
> (just to avoid confusing reviewers) but Jonathan asked me to remove the
> comment ;)

OK... ok... you convinced me :)

Thanks, will check soon your v3.
Andi