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:   Thu, 22 Sep 2022 20:39:48 +0000
From:   Sakari Ailus <sakari.ailus@linux.intel.com>
To:     Mikhail Rudenko <mike.rudenko@gmail.com>
Cc:     Mauro Carvalho Chehab <mchehab@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        Jacopo Mondi <jacopo@jmondi.org>,
        Shawn Tu <shawnx.tu@intel.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        Daniel Scally <djrscally@gmail.com>,
        Christian Hemp <c.hemp@phytec.de>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Marek Vasut <marex@denx.de>, linux-media@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] media: i2c: add support for ov4689
Message-ID: <YyzIFGo6SYhVP6sj@paasikivi.fi.intel.com>
References: <20220911200147.375198-1-mike.rudenko@gmail.com>
 <20220911200147.375198-3-mike.rudenko@gmail.com>
 <YywwjmN9qqwFoEc5@paasikivi.fi.intel.com>
 <87czbnif04.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87czbnif04.fsf@gmail.com>
Precedence: bulk

Hi Mikhail,

On Thu, Sep 22, 2022 at 06:23:05PM +0300, Mikhail Rudenko wrote:
> 
> Hi Sakari,
> 
> and thanks for reviewing this!

You're welcome!

> 
> Please see my comments below:
> 
> On 2022-09-22 at 09:53 GMT, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
...
> >> +static inline u32 ov4689_cal_delay(u32 cycles)
> >> +{
> >> +	return DIV_ROUND_UP(cycles, OV4689_XVCLK_FREQ / 1000 / 1000);
> >
> > Please use the actual rate instead.
> >
> 
> Do you mean clk_get_rate(ov4689->xvclk), right? What if we have an ACPI

Yes.

> system and xvclk is NULL here? Please explain.

There are a few ways this could work on ACPI based systems but generally
you'd have "clock-frequency" property to indicate the frequency even if
clocks themselves wouldn't be available (when controlled via ACPI)).

-- 
Kind regards,

Sakari Ailus