From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7BC87C433DB for ; Wed, 6 Jan 2021 15:54:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 53FAC2312D for ; Wed, 6 Jan 2021 15:54:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726430AbhAFPyk (ORCPT ); Wed, 6 Jan 2021 10:54:40 -0500 Received: from bhuna.collabora.co.uk ([46.235.227.227]:57826 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726206AbhAFPyk (ORCPT ); Wed, 6 Jan 2021 10:54:40 -0500 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: ezequiel) with ESMTPSA id 130E61F4604B Message-ID: <3950cb4b337e6373b066034c32e51a1e9e88a50f.camel@collabora.com> Subject: Re: [PATCH 1/6] media: mach-pxa: Register the camera sensor fixed-rate clock From: Ezequiel Garcia To: Petr Cvek , linux-media@vger.kernel.org, Hans Verkuil Cc: kernel@collabora.com, Arnd Bergmann , Robert Jarzmik , Janusz Krzysztofik , Sakari Ailus Date: Wed, 06 Jan 2021 12:53:50 -0300 In-Reply-To: References: <20210104165739.116404-1-ezequiel@collabora.com> <20210104165739.116404-2-ezequiel@collabora.com> Organization: Collabora Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.2-1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi Petr, Thanks a lot for reviewing and testing the series. On Tue, 2021-01-05 at 17:41 +0100, Petr Cvek wrote: > > Dne 04. 01. 21 v 17:57 Ezequiel Garcia napsal(a): > > The pxa-camera capture driver currently registers a v4l2-clk > > clock, named "mclk", to represent the mt9m111 sensor clock. > > > > Register a proper fixed-rate clock using the generic clock framework, > > which will allow to remove the v4l2-clk clock in the pxa-camera > > driver in a follow-up commit. > > > > BTW the mclk output to a sensor is actually a variable rate, divided from lcdclk (which can be changed too). PXA camera driver  is using variable > pcdev->mclk_divisor to generate the mclk from lcdclk. > Hm, now that I look at this, I see the pxa-camera driver is requiring a clock: pcdev->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(pcdev->clk)) return PTR_ERR(pcdev->clk); Where is this clock registered in the non-devicetree case? > The rate change is done in pxa_camera_activate(): > > https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/media/platform/pxa_camera.c#L1136 > >         __raw_writel(pcdev->mclk_divisor | cicr4, pcdev->base + CICR4); > > Would it be possible to register a correct clock type with possibility to change the divisor by the standard way? > Right, so you mean the pxa-camera driver is the one providing the clock for the sensors? In that case, I guess the pxa-camera driver should be the one registering a CCF clock. Other drivers are doing this, through clk_register for instance. However, for the sake of this series, which is meant to get rid of the v4l2-clk API, I would say it's fine to just register a fixed-rate. This is similar to what v4l2_clk_register was doing, which was registering a dummy clock. Having said that, as I mentioned above, I'm wondering if the mach-pxa boards are really working, given I'm not seeing the clock for pxa-camera. Maybe the best way forward is to just accept that pxa-camera is only supported for the device tree platforms, and therefore drop the support from mach-pxa/ boards. Thanks, Ezequiel > Petr > > > > Cc: Arnd Bergmann > > Cc: Robert Jarzmik > > Signed-off-by: Ezequiel Garcia > > --- > >  arch/arm/mach-pxa/devices.c | 8 ++++++++ > >  1 file changed, 8 insertions(+) > > > > diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c > > index 524d6093e0c7..09b8495f3fd9 100644 > > --- a/arch/arm/mach-pxa/devices.c > > +++ b/arch/arm/mach-pxa/devices.c > > @@ -4,6 +4,7 @@ > >  #include > >  #include > >  #include > > +#include > >  #include > >  #include > >  #include > > @@ -634,6 +635,13 @@ static struct platform_device pxa27x_device_camera = { > >   > >  void __init pxa_set_camera_info(struct pxacamera_platform_data *info) > >  { > > +       struct clk *mclk; > > + > > +       /* Register a fixed-rate clock for camera sensors. */ > > +       mclk = clk_register_fixed_rate(NULL, "pxa_camera_clk", NULL, 0, > > +                                            info->mclk_10khz * 10000); > > +       if (!IS_ERR(mclk)) > > +               clkdev_create(mclk, "mclk", NULL); > >         pxa_register_device(&pxa27x_device_camera, info); > >  } > >   > >