All of lore.kernel.org
 help / color / mirror / Atom feed
From: <Eugen.Hristev@microchip.com>
To: <jacopo@jmondi.org>
Cc: <linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<laurent.pinchart@ideasonboard.com>, <sakari.ailus@iki.fi>,
	<robh+dt@kernel.org>, <Nicolas.Ferre@microchip.com>
Subject: Re: [PATCH 05/21] media: atmel: atmel-isc: split the clock code into separate source file
Date: Fri, 5 Nov 2021 09:13:19 +0000	[thread overview]
Message-ID: <ab248578-cbd3-7ed4-2b5d-28d56048303b@microchip.com> (raw)
In-Reply-To: <20211105090204.sdiqdptsxmjs2ebr@uno.localdomain>

On 11/5/21 11:02 AM, Jacopo Mondi wrote:
> Hi Eugen
> 
> On Fri, Oct 22, 2021 at 10:52:31AM +0300, Eugen Hristev wrote:
>> The atmel-isc-base is getting crowded. Split the clock functions into
>> atmel-isc-clk.c.
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
>>   drivers/media/platform/atmel/Makefile         |   2 +-
>>   drivers/media/platform/atmel/atmel-isc-base.c | 294 ----------------
>>   drivers/media/platform/atmel/atmel-isc-clk.c  | 316 ++++++++++++++++++
>>   3 files changed, 317 insertions(+), 295 deletions(-)
>>   create mode 100644 drivers/media/platform/atmel/atmel-isc-clk.c
>>
>> diff --git a/drivers/media/platform/atmel/Makefile b/drivers/media/platform/atmel/Makefile
>> index 39f0a7eba702..1f6fe7427769 100644
>> --- a/drivers/media/platform/atmel/Makefile
>> +++ b/drivers/media/platform/atmel/Makefile
>> @@ -3,7 +3,7 @@ atmel-isc-objs = atmel-sama5d2-isc.o
>>   atmel-xisc-objs = atmel-sama7g5-isc.o
>>
>>   obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o
>> -obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += atmel-isc-base.o
>> +obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += atmel-isc-base.o atmel-isc-clk.o
>>   obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
>>   obj-$(CONFIG_VIDEO_ATMEL_XISC) += atmel-xisc.o
>>   obj-$(CONFIG_VIDEO_MICROCHIP_CSI2DC) += microchip-csi2dc.o
>> diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
>> index ebf264b980f9..f532fd03e807 100644
>> --- a/drivers/media/platform/atmel/atmel-isc-base.c
>> +++ b/drivers/media/platform/atmel/atmel-isc-base.c
>> @@ -8,9 +8,6 @@
>>    * Author: Eugen Hristev <eugen.hristev@microchip.com>
>>    *
>>    */
>> -
>> -#include <linux/clk.h>
>> -#include <linux/clkdev.h>
>>   #include <linux/clk-provider.h>
> 
> Do you still need clk-provider here ?

Yes it's needed in atmel-isc.h

> 
>>   #include <linux/delay.h>
>>   #include <linux/interrupt.h>
>> @@ -100,297 +97,6 @@ static inline void isc_reset_awb_ctrls(struct isc_device *isc)
>>        }
>>   }
>>
>> -static int isc_wait_clk_stable(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     struct regmap *regmap = isc_clk->regmap;
>> -     unsigned long timeout = jiffies + usecs_to_jiffies(1000);
>> -     unsigned int status;
>> -
>> -     while (time_before(jiffies, timeout)) {
>> -             regmap_read(regmap, ISC_CLKSR, &status);
>> -             if (!(status & ISC_CLKSR_SIP))
>> -                     return 0;
>> -
>> -             usleep_range(10, 250);
>> -     }
>> -
>> -     return -ETIMEDOUT;
>> -}
>> -
>> -static int isc_clk_prepare(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     int ret;
>> -
>> -     ret = pm_runtime_resume_and_get(isc_clk->dev);
>> -     if (ret < 0)
>> -             return ret;
>> -
>> -     return isc_wait_clk_stable(hw);
>> -}
>> -
>> -static void isc_clk_unprepare(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -
>> -     isc_wait_clk_stable(hw);
>> -
>> -     pm_runtime_put_sync(isc_clk->dev);
>> -}
>> -
>> -static int isc_clk_enable(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     u32 id = isc_clk->id;
>> -     struct regmap *regmap = isc_clk->regmap;
>> -     unsigned long flags;
>> -     unsigned int status;
>> -
>> -     dev_dbg(isc_clk->dev, "ISC CLK: %s, id = %d, div = %d, parent id = %d\n",
>> -             __func__, id, isc_clk->div, isc_clk->parent_id);
>> -
>> -     spin_lock_irqsave(&isc_clk->lock, flags);
>> -     regmap_update_bits(regmap, ISC_CLKCFG,
>> -                        ISC_CLKCFG_DIV_MASK(id) | ISC_CLKCFG_SEL_MASK(id),
>> -                        (isc_clk->div << ISC_CLKCFG_DIV_SHIFT(id)) |
>> -                        (isc_clk->parent_id << ISC_CLKCFG_SEL_SHIFT(id)));
>> -
>> -     regmap_write(regmap, ISC_CLKEN, ISC_CLK(id));
>> -     spin_unlock_irqrestore(&isc_clk->lock, flags);
>> -
>> -     regmap_read(regmap, ISC_CLKSR, &status);
>> -     if (status & ISC_CLK(id))
>> -             return 0;
>> -     else
>> -             return -EINVAL;
>> -}
>> -
>> -static void isc_clk_disable(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     u32 id = isc_clk->id;
>> -     unsigned long flags;
>> -
>> -     spin_lock_irqsave(&isc_clk->lock, flags);
>> -     regmap_write(isc_clk->regmap, ISC_CLKDIS, ISC_CLK(id));
>> -     spin_unlock_irqrestore(&isc_clk->lock, flags);
>> -}
>> -
>> -static int isc_clk_is_enabled(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     u32 status;
>> -     int ret;
>> -
>> -     ret = pm_runtime_resume_and_get(isc_clk->dev);
>> -     if (ret < 0)
>> -             return 0;
>> -
>> -     regmap_read(isc_clk->regmap, ISC_CLKSR, &status);
>> -
>> -     pm_runtime_put_sync(isc_clk->dev);
>> -
>> -     return status & ISC_CLK(isc_clk->id) ? 1 : 0;
>> -}
>> -
>> -static unsigned long
>> -isc_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -
>> -     return DIV_ROUND_CLOSEST(parent_rate, isc_clk->div + 1);
>> -}
>> -
>> -static int isc_clk_determine_rate(struct clk_hw *hw,
>> -                                struct clk_rate_request *req)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     long best_rate = -EINVAL;
>> -     int best_diff = -1;
>> -     unsigned int i, div;
>> -
>> -     for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
>> -             struct clk_hw *parent;
>> -             unsigned long parent_rate;
>> -
>> -             parent = clk_hw_get_parent_by_index(hw, i);
>> -             if (!parent)
>> -                     continue;
>> -
>> -             parent_rate = clk_hw_get_rate(parent);
>> -             if (!parent_rate)
>> -                     continue;
>> -
>> -             for (div = 1; div < ISC_CLK_MAX_DIV + 2; div++) {
>> -                     unsigned long rate;
>> -                     int diff;
>> -
>> -                     rate = DIV_ROUND_CLOSEST(parent_rate, div);
>> -                     diff = abs(req->rate - rate);
>> -
>> -                     if (best_diff < 0 || best_diff > diff) {
>> -                             best_rate = rate;
>> -                             best_diff = diff;
>> -                             req->best_parent_rate = parent_rate;
>> -                             req->best_parent_hw = parent;
>> -                     }
>> -
>> -                     if (!best_diff || rate < req->rate)
>> -                             break;
>> -             }
>> -
>> -             if (!best_diff)
>> -                     break;
>> -     }
>> -
>> -     dev_dbg(isc_clk->dev,
>> -             "ISC CLK: %s, best_rate = %ld, parent clk: %s @ %ld\n",
>> -             __func__, best_rate,
>> -             __clk_get_name((req->best_parent_hw)->clk),
>> -             req->best_parent_rate);
>> -
>> -     if (best_rate < 0)
>> -             return best_rate;
>> -
>> -     req->rate = best_rate;
>> -
>> -     return 0;
>> -}
>> -
>> -static int isc_clk_set_parent(struct clk_hw *hw, u8 index)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -
>> -     if (index >= clk_hw_get_num_parents(hw))
>> -             return -EINVAL;
>> -
>> -     isc_clk->parent_id = index;
>> -
>> -     return 0;
>> -}
>> -
>> -static u8 isc_clk_get_parent(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -
>> -     return isc_clk->parent_id;
>> -}
>> -
>> -static int isc_clk_set_rate(struct clk_hw *hw,
>> -                          unsigned long rate,
>> -                          unsigned long parent_rate)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     u32 div;
>> -
>> -     if (!rate)
>> -             return -EINVAL;
>> -
>> -     div = DIV_ROUND_CLOSEST(parent_rate, rate);
>> -     if (div > (ISC_CLK_MAX_DIV + 1) || !div)
>> -             return -EINVAL;
>> -
>> -     isc_clk->div = div - 1;
>> -
>> -     return 0;
>> -}
>> -
>> -static const struct clk_ops isc_clk_ops = {
>> -     .prepare        = isc_clk_prepare,
>> -     .unprepare      = isc_clk_unprepare,
>> -     .enable         = isc_clk_enable,
>> -     .disable        = isc_clk_disable,
>> -     .is_enabled     = isc_clk_is_enabled,
>> -     .recalc_rate    = isc_clk_recalc_rate,
>> -     .determine_rate = isc_clk_determine_rate,
>> -     .set_parent     = isc_clk_set_parent,
>> -     .get_parent     = isc_clk_get_parent,
>> -     .set_rate       = isc_clk_set_rate,
>> -};
>> -
>> -static int isc_clk_register(struct isc_device *isc, unsigned int id)
>> -{
>> -     struct regmap *regmap = isc->regmap;
>> -     struct device_node *np = isc->dev->of_node;
>> -     struct isc_clk *isc_clk;
>> -     struct clk_init_data init;
>> -     const char *clk_name = np->name;
>> -     const char *parent_names[3];
>> -     int num_parents;
>> -
>> -     if (id == ISC_ISPCK && !isc->ispck_required)
>> -             return 0;
>> -
>> -     num_parents = of_clk_get_parent_count(np);
>> -     if (num_parents < 1 || num_parents > 3)
>> -             return -EINVAL;
>> -
>> -     if (num_parents > 2 && id == ISC_ISPCK)
>> -             num_parents = 2;
>> -
>> -     of_clk_parent_fill(np, parent_names, num_parents);
>> -
>> -     if (id == ISC_MCK)
>> -             of_property_read_string(np, "clock-output-names", &clk_name);
>> -     else
>> -             clk_name = "isc-ispck";
>> -
>> -     init.parent_names       = parent_names;
>> -     init.num_parents        = num_parents;
>> -     init.name               = clk_name;
>> -     init.ops                = &isc_clk_ops;
>> -     init.flags              = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE;
>> -
>> -     isc_clk = &isc->isc_clks[id];
>> -     isc_clk->hw.init        = &init;
>> -     isc_clk->regmap         = regmap;
>> -     isc_clk->id             = id;
>> -     isc_clk->dev            = isc->dev;
>> -     spin_lock_init(&isc_clk->lock);
>> -
>> -     isc_clk->clk = clk_register(isc->dev, &isc_clk->hw);
>> -     if (IS_ERR(isc_clk->clk)) {
>> -             dev_err(isc->dev, "%s: clock register fail\n", clk_name);
>> -             return PTR_ERR(isc_clk->clk);
>> -     } else if (id == ISC_MCK)
>> -             of_clk_add_provider(np, of_clk_src_simple_get, isc_clk->clk);
>> -
>> -     return 0;
>> -}
>> -
>> -int isc_clk_init(struct isc_device *isc)
>> -{
>> -     unsigned int i;
>> -     int ret;
>> -
>> -     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++)
>> -             isc->isc_clks[i].clk = ERR_PTR(-EINVAL);
>> -
>> -     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
>> -             ret = isc_clk_register(isc, i);
>> -             if (ret)
>> -                     return ret;
>> -     }
>> -
>> -     return 0;
>> -}
>> -EXPORT_SYMBOL_GPL(isc_clk_init);
>> -
>> -void isc_clk_cleanup(struct isc_device *isc)
>> -{
>> -     unsigned int i;
>> -
>> -     of_clk_del_provider(isc->dev->of_node);
>> -
>> -     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
>> -             struct isc_clk *isc_clk = &isc->isc_clks[i];
>> -
>> -             if (!IS_ERR(isc_clk->clk))
>> -                     clk_unregister(isc_clk->clk);
>> -     }
>> -}
>> -EXPORT_SYMBOL_GPL(isc_clk_cleanup);
>>
>>   static int isc_queue_setup(struct vb2_queue *vq,
>>                            unsigned int *nbuffers, unsigned int *nplanes,
>> diff --git a/drivers/media/platform/atmel/atmel-isc-clk.c b/drivers/media/platform/atmel/atmel-isc-clk.c
>> new file mode 100644
>> index 000000000000..d650caade396
>> --- /dev/null
>> +++ b/drivers/media/platform/atmel/atmel-isc-clk.c
>> @@ -0,0 +1,316 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Microchip Image Sensor Controller (ISC) common clock driver setup
>> + *
>> + * Copyright (C) 2016-2019 Microchip Technology, Inc.
> 
> Time flies!

I think I will change this to 2016 only, to keep just the original 
driver's first year of inclusion

> 
>> + *
>> + * Author: Songjun Wu
>> + * Author: Eugen Hristev <eugen.hristev@microchip.com>
>> + *
>> + */
>> +#include <linux/clk.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
> 
> Is this needed ?
> 
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/videobuf2-dma-contig.h>
> 
> Is any one of these needed ?
> 
> Removing them highlights how these includes should probably be
> moved to atmel-isc.h which fails to compile if not preceeded by these
> inclusions.

Yes, this is the reason why the headers are there. I will try to move 
them to atmel-isc.h

> 
> I think you can merge this with the previous patch that adds the file
> entry to MAINTAINERS

I usually avoid to do that, because of the usual mess with merging 
patches on MAINTAINERS. So rather make it easy and clean with a simple 
patch directly to MAINTAINERS.

But if you think it's better to have it together with the driver change, 
I can squash it

Thank you for having a look at this,
Eugen
> 
> Thanks
>     j
> 
>> +
>> +#include "atmel-isc-regs.h"
>> +#include "atmel-isc.h"
>> +
>> +static int isc_wait_clk_stable(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     struct regmap *regmap = isc_clk->regmap;
>> +     unsigned long timeout = jiffies + usecs_to_jiffies(1000);
>> +     unsigned int status;
>> +
>> +     while (time_before(jiffies, timeout)) {
>> +             regmap_read(regmap, ISC_CLKSR, &status);
>> +             if (!(status & ISC_CLKSR_SIP))
>> +                     return 0;
>> +
>> +             usleep_range(10, 250);
>> +     }
>> +
>> +     return -ETIMEDOUT;
>> +}
>> +
>> +static int isc_clk_prepare(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     int ret;
>> +
>> +     ret = pm_runtime_resume_and_get(isc_clk->dev);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     return isc_wait_clk_stable(hw);
>> +}
>> +
>> +static void isc_clk_unprepare(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +
>> +     isc_wait_clk_stable(hw);
>> +
>> +     pm_runtime_put_sync(isc_clk->dev);
>> +}
>> +
>> +static int isc_clk_enable(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     u32 id = isc_clk->id;
>> +     struct regmap *regmap = isc_clk->regmap;
>> +     unsigned long flags;
>> +     unsigned int status;
>> +
>> +     dev_dbg(isc_clk->dev, "ISC CLK: %s, id = %d, div = %d, parent id = %d\n",
>> +             __func__, id, isc_clk->div, isc_clk->parent_id);
>> +
>> +     spin_lock_irqsave(&isc_clk->lock, flags);
>> +     regmap_update_bits(regmap, ISC_CLKCFG,
>> +                        ISC_CLKCFG_DIV_MASK(id) | ISC_CLKCFG_SEL_MASK(id),
>> +                        (isc_clk->div << ISC_CLKCFG_DIV_SHIFT(id)) |
>> +                        (isc_clk->parent_id << ISC_CLKCFG_SEL_SHIFT(id)));
>> +
>> +     regmap_write(regmap, ISC_CLKEN, ISC_CLK(id));
>> +     spin_unlock_irqrestore(&isc_clk->lock, flags);
>> +
>> +     regmap_read(regmap, ISC_CLKSR, &status);
>> +     if (status & ISC_CLK(id))
>> +             return 0;
>> +     else
>> +             return -EINVAL;
>> +}
>> +
>> +static void isc_clk_disable(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     u32 id = isc_clk->id;
>> +     unsigned long flags;
>> +
>> +     spin_lock_irqsave(&isc_clk->lock, flags);
>> +     regmap_write(isc_clk->regmap, ISC_CLKDIS, ISC_CLK(id));
>> +     spin_unlock_irqrestore(&isc_clk->lock, flags);
>> +}
>> +
>> +static int isc_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     u32 status;
>> +     int ret;
>> +
>> +     ret = pm_runtime_resume_and_get(isc_clk->dev);
>> +     if (ret < 0)
>> +             return 0;
>> +
>> +     regmap_read(isc_clk->regmap, ISC_CLKSR, &status);
>> +
>> +     pm_runtime_put_sync(isc_clk->dev);
>> +
>> +     return status & ISC_CLK(isc_clk->id) ? 1 : 0;
>> +}
>> +
>> +static unsigned long
>> +isc_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +
>> +     return DIV_ROUND_CLOSEST(parent_rate, isc_clk->div + 1);
>> +}
>> +
>> +static int isc_clk_determine_rate(struct clk_hw *hw,
>> +                               struct clk_rate_request *req)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     long best_rate = -EINVAL;
>> +     int best_diff = -1;
>> +     unsigned int i, div;
>> +
>> +     for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
>> +             struct clk_hw *parent;
>> +             unsigned long parent_rate;
>> +
>> +             parent = clk_hw_get_parent_by_index(hw, i);
>> +             if (!parent)
>> +                     continue;
>> +
>> +             parent_rate = clk_hw_get_rate(parent);
>> +             if (!parent_rate)
>> +                     continue;
>> +
>> +             for (div = 1; div < ISC_CLK_MAX_DIV + 2; div++) {
>> +                     unsigned long rate;
>> +                     int diff;
>> +
>> +                     rate = DIV_ROUND_CLOSEST(parent_rate, div);
>> +                     diff = abs(req->rate - rate);
>> +
>> +                     if (best_diff < 0 || best_diff > diff) {
>> +                             best_rate = rate;
>> +                             best_diff = diff;
>> +                             req->best_parent_rate = parent_rate;
>> +                             req->best_parent_hw = parent;
>> +                     }
>> +
>> +                     if (!best_diff || rate < req->rate)
>> +                             break;
>> +             }
>> +
>> +             if (!best_diff)
>> +                     break;
>> +     }
>> +
>> +     dev_dbg(isc_clk->dev,
>> +             "ISC CLK: %s, best_rate = %ld, parent clk: %s @ %ld\n",
>> +             __func__, best_rate,
>> +             __clk_get_name((req->best_parent_hw)->clk),
>> +             req->best_parent_rate);
>> +
>> +     if (best_rate < 0)
>> +             return best_rate;
>> +
>> +     req->rate = best_rate;
>> +
>> +     return 0;
>> +}
>> +
>> +static int isc_clk_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +
>> +     if (index >= clk_hw_get_num_parents(hw))
>> +             return -EINVAL;
>> +
>> +     isc_clk->parent_id = index;
>> +
>> +     return 0;
>> +}
>> +
>> +static u8 isc_clk_get_parent(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +
>> +     return isc_clk->parent_id;
>> +}
>> +
>> +static int isc_clk_set_rate(struct clk_hw *hw,
>> +                         unsigned long rate,
>> +                         unsigned long parent_rate)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     u32 div;
>> +
>> +     if (!rate)
>> +             return -EINVAL;
>> +
>> +     div = DIV_ROUND_CLOSEST(parent_rate, rate);
>> +     if (div > (ISC_CLK_MAX_DIV + 1) || !div)
>> +             return -EINVAL;
>> +
>> +     isc_clk->div = div - 1;
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct clk_ops isc_clk_ops = {
>> +     .prepare        = isc_clk_prepare,
>> +     .unprepare      = isc_clk_unprepare,
>> +     .enable         = isc_clk_enable,
>> +     .disable        = isc_clk_disable,
>> +     .is_enabled     = isc_clk_is_enabled,
>> +     .recalc_rate    = isc_clk_recalc_rate,
>> +     .determine_rate = isc_clk_determine_rate,
>> +     .set_parent     = isc_clk_set_parent,
>> +     .get_parent     = isc_clk_get_parent,
>> +     .set_rate       = isc_clk_set_rate,
>> +};
>> +
>> +static int isc_clk_register(struct isc_device *isc, unsigned int id)
>> +{
>> +     struct regmap *regmap = isc->regmap;
>> +     struct device_node *np = isc->dev->of_node;
>> +     struct isc_clk *isc_clk;
>> +     struct clk_init_data init;
>> +     const char *clk_name = np->name;
>> +     const char *parent_names[3];
>> +     int num_parents;
>> +
>> +     if (id == ISC_ISPCK && !isc->ispck_required)
>> +             return 0;
>> +
>> +     num_parents = of_clk_get_parent_count(np);
>> +     if (num_parents < 1 || num_parents > 3)
>> +             return -EINVAL;
>> +
>> +     if (num_parents > 2 && id == ISC_ISPCK)
>> +             num_parents = 2;
>> +
>> +     of_clk_parent_fill(np, parent_names, num_parents);
>> +
>> +     if (id == ISC_MCK)
>> +             of_property_read_string(np, "clock-output-names", &clk_name);
>> +     else
>> +             clk_name = "isc-ispck";
>> +
>> +     init.parent_names       = parent_names;
>> +     init.num_parents        = num_parents;
>> +     init.name               = clk_name;
>> +     init.ops                = &isc_clk_ops;
>> +     init.flags              = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE;
>> +
>> +     isc_clk = &isc->isc_clks[id];
>> +     isc_clk->hw.init        = &init;
>> +     isc_clk->regmap         = regmap;
>> +     isc_clk->id             = id;
>> +     isc_clk->dev            = isc->dev;
>> +     spin_lock_init(&isc_clk->lock);
>> +
>> +     isc_clk->clk = clk_register(isc->dev, &isc_clk->hw);
>> +     if (IS_ERR(isc_clk->clk)) {
>> +             dev_err(isc->dev, "%s: clock register fail\n", clk_name);
>> +             return PTR_ERR(isc_clk->clk);
>> +     } else if (id == ISC_MCK) {
>> +             of_clk_add_provider(np, of_clk_src_simple_get, isc_clk->clk);
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +int isc_clk_init(struct isc_device *isc)
>> +{
>> +     unsigned int i;
>> +     int ret;
>> +
>> +     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++)
>> +             isc->isc_clks[i].clk = ERR_PTR(-EINVAL);
>> +
>> +     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
>> +             ret = isc_clk_register(isc, i);
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(isc_clk_init);
>> +
>> +void isc_clk_cleanup(struct isc_device *isc)
>> +{
>> +     unsigned int i;
>> +
>> +     of_clk_del_provider(isc->dev->of_node);
>> +
>> +     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
>> +             struct isc_clk *isc_clk = &isc->isc_clks[i];
>> +
>> +             if (!IS_ERR(isc_clk->clk))
>> +                     clk_unregister(isc_clk->clk);
>> +     }
>> +}
>> +EXPORT_SYMBOL_GPL(isc_clk_cleanup);
>> --
>> 2.25.1
>>


WARNING: multiple messages have this Message-ID (diff)
From: <Eugen.Hristev@microchip.com>
To: <jacopo@jmondi.org>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	robh+dt@kernel.org, sakari.ailus@iki.fi,
	laurent.pinchart@ideasonboard.com,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH 05/21] media: atmel: atmel-isc: split the clock code into separate source file
Date: Fri, 5 Nov 2021 09:13:19 +0000	[thread overview]
Message-ID: <ab248578-cbd3-7ed4-2b5d-28d56048303b@microchip.com> (raw)
In-Reply-To: <20211105090204.sdiqdptsxmjs2ebr@uno.localdomain>

On 11/5/21 11:02 AM, Jacopo Mondi wrote:
> Hi Eugen
> 
> On Fri, Oct 22, 2021 at 10:52:31AM +0300, Eugen Hristev wrote:
>> The atmel-isc-base is getting crowded. Split the clock functions into
>> atmel-isc-clk.c.
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
>>   drivers/media/platform/atmel/Makefile         |   2 +-
>>   drivers/media/platform/atmel/atmel-isc-base.c | 294 ----------------
>>   drivers/media/platform/atmel/atmel-isc-clk.c  | 316 ++++++++++++++++++
>>   3 files changed, 317 insertions(+), 295 deletions(-)
>>   create mode 100644 drivers/media/platform/atmel/atmel-isc-clk.c
>>
>> diff --git a/drivers/media/platform/atmel/Makefile b/drivers/media/platform/atmel/Makefile
>> index 39f0a7eba702..1f6fe7427769 100644
>> --- a/drivers/media/platform/atmel/Makefile
>> +++ b/drivers/media/platform/atmel/Makefile
>> @@ -3,7 +3,7 @@ atmel-isc-objs = atmel-sama5d2-isc.o
>>   atmel-xisc-objs = atmel-sama7g5-isc.o
>>
>>   obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o
>> -obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += atmel-isc-base.o
>> +obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += atmel-isc-base.o atmel-isc-clk.o
>>   obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
>>   obj-$(CONFIG_VIDEO_ATMEL_XISC) += atmel-xisc.o
>>   obj-$(CONFIG_VIDEO_MICROCHIP_CSI2DC) += microchip-csi2dc.o
>> diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
>> index ebf264b980f9..f532fd03e807 100644
>> --- a/drivers/media/platform/atmel/atmel-isc-base.c
>> +++ b/drivers/media/platform/atmel/atmel-isc-base.c
>> @@ -8,9 +8,6 @@
>>    * Author: Eugen Hristev <eugen.hristev@microchip.com>
>>    *
>>    */
>> -
>> -#include <linux/clk.h>
>> -#include <linux/clkdev.h>
>>   #include <linux/clk-provider.h>
> 
> Do you still need clk-provider here ?

Yes it's needed in atmel-isc.h

> 
>>   #include <linux/delay.h>
>>   #include <linux/interrupt.h>
>> @@ -100,297 +97,6 @@ static inline void isc_reset_awb_ctrls(struct isc_device *isc)
>>        }
>>   }
>>
>> -static int isc_wait_clk_stable(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     struct regmap *regmap = isc_clk->regmap;
>> -     unsigned long timeout = jiffies + usecs_to_jiffies(1000);
>> -     unsigned int status;
>> -
>> -     while (time_before(jiffies, timeout)) {
>> -             regmap_read(regmap, ISC_CLKSR, &status);
>> -             if (!(status & ISC_CLKSR_SIP))
>> -                     return 0;
>> -
>> -             usleep_range(10, 250);
>> -     }
>> -
>> -     return -ETIMEDOUT;
>> -}
>> -
>> -static int isc_clk_prepare(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     int ret;
>> -
>> -     ret = pm_runtime_resume_and_get(isc_clk->dev);
>> -     if (ret < 0)
>> -             return ret;
>> -
>> -     return isc_wait_clk_stable(hw);
>> -}
>> -
>> -static void isc_clk_unprepare(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -
>> -     isc_wait_clk_stable(hw);
>> -
>> -     pm_runtime_put_sync(isc_clk->dev);
>> -}
>> -
>> -static int isc_clk_enable(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     u32 id = isc_clk->id;
>> -     struct regmap *regmap = isc_clk->regmap;
>> -     unsigned long flags;
>> -     unsigned int status;
>> -
>> -     dev_dbg(isc_clk->dev, "ISC CLK: %s, id = %d, div = %d, parent id = %d\n",
>> -             __func__, id, isc_clk->div, isc_clk->parent_id);
>> -
>> -     spin_lock_irqsave(&isc_clk->lock, flags);
>> -     regmap_update_bits(regmap, ISC_CLKCFG,
>> -                        ISC_CLKCFG_DIV_MASK(id) | ISC_CLKCFG_SEL_MASK(id),
>> -                        (isc_clk->div << ISC_CLKCFG_DIV_SHIFT(id)) |
>> -                        (isc_clk->parent_id << ISC_CLKCFG_SEL_SHIFT(id)));
>> -
>> -     regmap_write(regmap, ISC_CLKEN, ISC_CLK(id));
>> -     spin_unlock_irqrestore(&isc_clk->lock, flags);
>> -
>> -     regmap_read(regmap, ISC_CLKSR, &status);
>> -     if (status & ISC_CLK(id))
>> -             return 0;
>> -     else
>> -             return -EINVAL;
>> -}
>> -
>> -static void isc_clk_disable(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     u32 id = isc_clk->id;
>> -     unsigned long flags;
>> -
>> -     spin_lock_irqsave(&isc_clk->lock, flags);
>> -     regmap_write(isc_clk->regmap, ISC_CLKDIS, ISC_CLK(id));
>> -     spin_unlock_irqrestore(&isc_clk->lock, flags);
>> -}
>> -
>> -static int isc_clk_is_enabled(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     u32 status;
>> -     int ret;
>> -
>> -     ret = pm_runtime_resume_and_get(isc_clk->dev);
>> -     if (ret < 0)
>> -             return 0;
>> -
>> -     regmap_read(isc_clk->regmap, ISC_CLKSR, &status);
>> -
>> -     pm_runtime_put_sync(isc_clk->dev);
>> -
>> -     return status & ISC_CLK(isc_clk->id) ? 1 : 0;
>> -}
>> -
>> -static unsigned long
>> -isc_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -
>> -     return DIV_ROUND_CLOSEST(parent_rate, isc_clk->div + 1);
>> -}
>> -
>> -static int isc_clk_determine_rate(struct clk_hw *hw,
>> -                                struct clk_rate_request *req)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     long best_rate = -EINVAL;
>> -     int best_diff = -1;
>> -     unsigned int i, div;
>> -
>> -     for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
>> -             struct clk_hw *parent;
>> -             unsigned long parent_rate;
>> -
>> -             parent = clk_hw_get_parent_by_index(hw, i);
>> -             if (!parent)
>> -                     continue;
>> -
>> -             parent_rate = clk_hw_get_rate(parent);
>> -             if (!parent_rate)
>> -                     continue;
>> -
>> -             for (div = 1; div < ISC_CLK_MAX_DIV + 2; div++) {
>> -                     unsigned long rate;
>> -                     int diff;
>> -
>> -                     rate = DIV_ROUND_CLOSEST(parent_rate, div);
>> -                     diff = abs(req->rate - rate);
>> -
>> -                     if (best_diff < 0 || best_diff > diff) {
>> -                             best_rate = rate;
>> -                             best_diff = diff;
>> -                             req->best_parent_rate = parent_rate;
>> -                             req->best_parent_hw = parent;
>> -                     }
>> -
>> -                     if (!best_diff || rate < req->rate)
>> -                             break;
>> -             }
>> -
>> -             if (!best_diff)
>> -                     break;
>> -     }
>> -
>> -     dev_dbg(isc_clk->dev,
>> -             "ISC CLK: %s, best_rate = %ld, parent clk: %s @ %ld\n",
>> -             __func__, best_rate,
>> -             __clk_get_name((req->best_parent_hw)->clk),
>> -             req->best_parent_rate);
>> -
>> -     if (best_rate < 0)
>> -             return best_rate;
>> -
>> -     req->rate = best_rate;
>> -
>> -     return 0;
>> -}
>> -
>> -static int isc_clk_set_parent(struct clk_hw *hw, u8 index)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -
>> -     if (index >= clk_hw_get_num_parents(hw))
>> -             return -EINVAL;
>> -
>> -     isc_clk->parent_id = index;
>> -
>> -     return 0;
>> -}
>> -
>> -static u8 isc_clk_get_parent(struct clk_hw *hw)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -
>> -     return isc_clk->parent_id;
>> -}
>> -
>> -static int isc_clk_set_rate(struct clk_hw *hw,
>> -                          unsigned long rate,
>> -                          unsigned long parent_rate)
>> -{
>> -     struct isc_clk *isc_clk = to_isc_clk(hw);
>> -     u32 div;
>> -
>> -     if (!rate)
>> -             return -EINVAL;
>> -
>> -     div = DIV_ROUND_CLOSEST(parent_rate, rate);
>> -     if (div > (ISC_CLK_MAX_DIV + 1) || !div)
>> -             return -EINVAL;
>> -
>> -     isc_clk->div = div - 1;
>> -
>> -     return 0;
>> -}
>> -
>> -static const struct clk_ops isc_clk_ops = {
>> -     .prepare        = isc_clk_prepare,
>> -     .unprepare      = isc_clk_unprepare,
>> -     .enable         = isc_clk_enable,
>> -     .disable        = isc_clk_disable,
>> -     .is_enabled     = isc_clk_is_enabled,
>> -     .recalc_rate    = isc_clk_recalc_rate,
>> -     .determine_rate = isc_clk_determine_rate,
>> -     .set_parent     = isc_clk_set_parent,
>> -     .get_parent     = isc_clk_get_parent,
>> -     .set_rate       = isc_clk_set_rate,
>> -};
>> -
>> -static int isc_clk_register(struct isc_device *isc, unsigned int id)
>> -{
>> -     struct regmap *regmap = isc->regmap;
>> -     struct device_node *np = isc->dev->of_node;
>> -     struct isc_clk *isc_clk;
>> -     struct clk_init_data init;
>> -     const char *clk_name = np->name;
>> -     const char *parent_names[3];
>> -     int num_parents;
>> -
>> -     if (id == ISC_ISPCK && !isc->ispck_required)
>> -             return 0;
>> -
>> -     num_parents = of_clk_get_parent_count(np);
>> -     if (num_parents < 1 || num_parents > 3)
>> -             return -EINVAL;
>> -
>> -     if (num_parents > 2 && id == ISC_ISPCK)
>> -             num_parents = 2;
>> -
>> -     of_clk_parent_fill(np, parent_names, num_parents);
>> -
>> -     if (id == ISC_MCK)
>> -             of_property_read_string(np, "clock-output-names", &clk_name);
>> -     else
>> -             clk_name = "isc-ispck";
>> -
>> -     init.parent_names       = parent_names;
>> -     init.num_parents        = num_parents;
>> -     init.name               = clk_name;
>> -     init.ops                = &isc_clk_ops;
>> -     init.flags              = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE;
>> -
>> -     isc_clk = &isc->isc_clks[id];
>> -     isc_clk->hw.init        = &init;
>> -     isc_clk->regmap         = regmap;
>> -     isc_clk->id             = id;
>> -     isc_clk->dev            = isc->dev;
>> -     spin_lock_init(&isc_clk->lock);
>> -
>> -     isc_clk->clk = clk_register(isc->dev, &isc_clk->hw);
>> -     if (IS_ERR(isc_clk->clk)) {
>> -             dev_err(isc->dev, "%s: clock register fail\n", clk_name);
>> -             return PTR_ERR(isc_clk->clk);
>> -     } else if (id == ISC_MCK)
>> -             of_clk_add_provider(np, of_clk_src_simple_get, isc_clk->clk);
>> -
>> -     return 0;
>> -}
>> -
>> -int isc_clk_init(struct isc_device *isc)
>> -{
>> -     unsigned int i;
>> -     int ret;
>> -
>> -     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++)
>> -             isc->isc_clks[i].clk = ERR_PTR(-EINVAL);
>> -
>> -     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
>> -             ret = isc_clk_register(isc, i);
>> -             if (ret)
>> -                     return ret;
>> -     }
>> -
>> -     return 0;
>> -}
>> -EXPORT_SYMBOL_GPL(isc_clk_init);
>> -
>> -void isc_clk_cleanup(struct isc_device *isc)
>> -{
>> -     unsigned int i;
>> -
>> -     of_clk_del_provider(isc->dev->of_node);
>> -
>> -     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
>> -             struct isc_clk *isc_clk = &isc->isc_clks[i];
>> -
>> -             if (!IS_ERR(isc_clk->clk))
>> -                     clk_unregister(isc_clk->clk);
>> -     }
>> -}
>> -EXPORT_SYMBOL_GPL(isc_clk_cleanup);
>>
>>   static int isc_queue_setup(struct vb2_queue *vq,
>>                            unsigned int *nbuffers, unsigned int *nplanes,
>> diff --git a/drivers/media/platform/atmel/atmel-isc-clk.c b/drivers/media/platform/atmel/atmel-isc-clk.c
>> new file mode 100644
>> index 000000000000..d650caade396
>> --- /dev/null
>> +++ b/drivers/media/platform/atmel/atmel-isc-clk.c
>> @@ -0,0 +1,316 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Microchip Image Sensor Controller (ISC) common clock driver setup
>> + *
>> + * Copyright (C) 2016-2019 Microchip Technology, Inc.
> 
> Time flies!

I think I will change this to 2016 only, to keep just the original 
driver's first year of inclusion

> 
>> + *
>> + * Author: Songjun Wu
>> + * Author: Eugen Hristev <eugen.hristev@microchip.com>
>> + *
>> + */
>> +#include <linux/clk.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
> 
> Is this needed ?
> 
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/videobuf2-dma-contig.h>
> 
> Is any one of these needed ?
> 
> Removing them highlights how these includes should probably be
> moved to atmel-isc.h which fails to compile if not preceeded by these
> inclusions.

Yes, this is the reason why the headers are there. I will try to move 
them to atmel-isc.h

> 
> I think you can merge this with the previous patch that adds the file
> entry to MAINTAINERS

I usually avoid to do that, because of the usual mess with merging 
patches on MAINTAINERS. So rather make it easy and clean with a simple 
patch directly to MAINTAINERS.

But if you think it's better to have it together with the driver change, 
I can squash it

Thank you for having a look at this,
Eugen
> 
> Thanks
>     j
> 
>> +
>> +#include "atmel-isc-regs.h"
>> +#include "atmel-isc.h"
>> +
>> +static int isc_wait_clk_stable(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     struct regmap *regmap = isc_clk->regmap;
>> +     unsigned long timeout = jiffies + usecs_to_jiffies(1000);
>> +     unsigned int status;
>> +
>> +     while (time_before(jiffies, timeout)) {
>> +             regmap_read(regmap, ISC_CLKSR, &status);
>> +             if (!(status & ISC_CLKSR_SIP))
>> +                     return 0;
>> +
>> +             usleep_range(10, 250);
>> +     }
>> +
>> +     return -ETIMEDOUT;
>> +}
>> +
>> +static int isc_clk_prepare(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     int ret;
>> +
>> +     ret = pm_runtime_resume_and_get(isc_clk->dev);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     return isc_wait_clk_stable(hw);
>> +}
>> +
>> +static void isc_clk_unprepare(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +
>> +     isc_wait_clk_stable(hw);
>> +
>> +     pm_runtime_put_sync(isc_clk->dev);
>> +}
>> +
>> +static int isc_clk_enable(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     u32 id = isc_clk->id;
>> +     struct regmap *regmap = isc_clk->regmap;
>> +     unsigned long flags;
>> +     unsigned int status;
>> +
>> +     dev_dbg(isc_clk->dev, "ISC CLK: %s, id = %d, div = %d, parent id = %d\n",
>> +             __func__, id, isc_clk->div, isc_clk->parent_id);
>> +
>> +     spin_lock_irqsave(&isc_clk->lock, flags);
>> +     regmap_update_bits(regmap, ISC_CLKCFG,
>> +                        ISC_CLKCFG_DIV_MASK(id) | ISC_CLKCFG_SEL_MASK(id),
>> +                        (isc_clk->div << ISC_CLKCFG_DIV_SHIFT(id)) |
>> +                        (isc_clk->parent_id << ISC_CLKCFG_SEL_SHIFT(id)));
>> +
>> +     regmap_write(regmap, ISC_CLKEN, ISC_CLK(id));
>> +     spin_unlock_irqrestore(&isc_clk->lock, flags);
>> +
>> +     regmap_read(regmap, ISC_CLKSR, &status);
>> +     if (status & ISC_CLK(id))
>> +             return 0;
>> +     else
>> +             return -EINVAL;
>> +}
>> +
>> +static void isc_clk_disable(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     u32 id = isc_clk->id;
>> +     unsigned long flags;
>> +
>> +     spin_lock_irqsave(&isc_clk->lock, flags);
>> +     regmap_write(isc_clk->regmap, ISC_CLKDIS, ISC_CLK(id));
>> +     spin_unlock_irqrestore(&isc_clk->lock, flags);
>> +}
>> +
>> +static int isc_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     u32 status;
>> +     int ret;
>> +
>> +     ret = pm_runtime_resume_and_get(isc_clk->dev);
>> +     if (ret < 0)
>> +             return 0;
>> +
>> +     regmap_read(isc_clk->regmap, ISC_CLKSR, &status);
>> +
>> +     pm_runtime_put_sync(isc_clk->dev);
>> +
>> +     return status & ISC_CLK(isc_clk->id) ? 1 : 0;
>> +}
>> +
>> +static unsigned long
>> +isc_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +
>> +     return DIV_ROUND_CLOSEST(parent_rate, isc_clk->div + 1);
>> +}
>> +
>> +static int isc_clk_determine_rate(struct clk_hw *hw,
>> +                               struct clk_rate_request *req)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     long best_rate = -EINVAL;
>> +     int best_diff = -1;
>> +     unsigned int i, div;
>> +
>> +     for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
>> +             struct clk_hw *parent;
>> +             unsigned long parent_rate;
>> +
>> +             parent = clk_hw_get_parent_by_index(hw, i);
>> +             if (!parent)
>> +                     continue;
>> +
>> +             parent_rate = clk_hw_get_rate(parent);
>> +             if (!parent_rate)
>> +                     continue;
>> +
>> +             for (div = 1; div < ISC_CLK_MAX_DIV + 2; div++) {
>> +                     unsigned long rate;
>> +                     int diff;
>> +
>> +                     rate = DIV_ROUND_CLOSEST(parent_rate, div);
>> +                     diff = abs(req->rate - rate);
>> +
>> +                     if (best_diff < 0 || best_diff > diff) {
>> +                             best_rate = rate;
>> +                             best_diff = diff;
>> +                             req->best_parent_rate = parent_rate;
>> +                             req->best_parent_hw = parent;
>> +                     }
>> +
>> +                     if (!best_diff || rate < req->rate)
>> +                             break;
>> +             }
>> +
>> +             if (!best_diff)
>> +                     break;
>> +     }
>> +
>> +     dev_dbg(isc_clk->dev,
>> +             "ISC CLK: %s, best_rate = %ld, parent clk: %s @ %ld\n",
>> +             __func__, best_rate,
>> +             __clk_get_name((req->best_parent_hw)->clk),
>> +             req->best_parent_rate);
>> +
>> +     if (best_rate < 0)
>> +             return best_rate;
>> +
>> +     req->rate = best_rate;
>> +
>> +     return 0;
>> +}
>> +
>> +static int isc_clk_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +
>> +     if (index >= clk_hw_get_num_parents(hw))
>> +             return -EINVAL;
>> +
>> +     isc_clk->parent_id = index;
>> +
>> +     return 0;
>> +}
>> +
>> +static u8 isc_clk_get_parent(struct clk_hw *hw)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +
>> +     return isc_clk->parent_id;
>> +}
>> +
>> +static int isc_clk_set_rate(struct clk_hw *hw,
>> +                         unsigned long rate,
>> +                         unsigned long parent_rate)
>> +{
>> +     struct isc_clk *isc_clk = to_isc_clk(hw);
>> +     u32 div;
>> +
>> +     if (!rate)
>> +             return -EINVAL;
>> +
>> +     div = DIV_ROUND_CLOSEST(parent_rate, rate);
>> +     if (div > (ISC_CLK_MAX_DIV + 1) || !div)
>> +             return -EINVAL;
>> +
>> +     isc_clk->div = div - 1;
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct clk_ops isc_clk_ops = {
>> +     .prepare        = isc_clk_prepare,
>> +     .unprepare      = isc_clk_unprepare,
>> +     .enable         = isc_clk_enable,
>> +     .disable        = isc_clk_disable,
>> +     .is_enabled     = isc_clk_is_enabled,
>> +     .recalc_rate    = isc_clk_recalc_rate,
>> +     .determine_rate = isc_clk_determine_rate,
>> +     .set_parent     = isc_clk_set_parent,
>> +     .get_parent     = isc_clk_get_parent,
>> +     .set_rate       = isc_clk_set_rate,
>> +};
>> +
>> +static int isc_clk_register(struct isc_device *isc, unsigned int id)
>> +{
>> +     struct regmap *regmap = isc->regmap;
>> +     struct device_node *np = isc->dev->of_node;
>> +     struct isc_clk *isc_clk;
>> +     struct clk_init_data init;
>> +     const char *clk_name = np->name;
>> +     const char *parent_names[3];
>> +     int num_parents;
>> +
>> +     if (id == ISC_ISPCK && !isc->ispck_required)
>> +             return 0;
>> +
>> +     num_parents = of_clk_get_parent_count(np);
>> +     if (num_parents < 1 || num_parents > 3)
>> +             return -EINVAL;
>> +
>> +     if (num_parents > 2 && id == ISC_ISPCK)
>> +             num_parents = 2;
>> +
>> +     of_clk_parent_fill(np, parent_names, num_parents);
>> +
>> +     if (id == ISC_MCK)
>> +             of_property_read_string(np, "clock-output-names", &clk_name);
>> +     else
>> +             clk_name = "isc-ispck";
>> +
>> +     init.parent_names       = parent_names;
>> +     init.num_parents        = num_parents;
>> +     init.name               = clk_name;
>> +     init.ops                = &isc_clk_ops;
>> +     init.flags              = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE;
>> +
>> +     isc_clk = &isc->isc_clks[id];
>> +     isc_clk->hw.init        = &init;
>> +     isc_clk->regmap         = regmap;
>> +     isc_clk->id             = id;
>> +     isc_clk->dev            = isc->dev;
>> +     spin_lock_init(&isc_clk->lock);
>> +
>> +     isc_clk->clk = clk_register(isc->dev, &isc_clk->hw);
>> +     if (IS_ERR(isc_clk->clk)) {
>> +             dev_err(isc->dev, "%s: clock register fail\n", clk_name);
>> +             return PTR_ERR(isc_clk->clk);
>> +     } else if (id == ISC_MCK) {
>> +             of_clk_add_provider(np, of_clk_src_simple_get, isc_clk->clk);
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +int isc_clk_init(struct isc_device *isc)
>> +{
>> +     unsigned int i;
>> +     int ret;
>> +
>> +     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++)
>> +             isc->isc_clks[i].clk = ERR_PTR(-EINVAL);
>> +
>> +     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
>> +             ret = isc_clk_register(isc, i);
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(isc_clk_init);
>> +
>> +void isc_clk_cleanup(struct isc_device *isc)
>> +{
>> +     unsigned int i;
>> +
>> +     of_clk_del_provider(isc->dev->of_node);
>> +
>> +     for (i = 0; i < ARRAY_SIZE(isc->isc_clks); i++) {
>> +             struct isc_clk *isc_clk = &isc->isc_clks[i];
>> +
>> +             if (!IS_ERR(isc_clk->clk))
>> +                     clk_unregister(isc_clk->clk);
>> +     }
>> +}
>> +EXPORT_SYMBOL_GPL(isc_clk_cleanup);
>> --
>> 2.25.1
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-11-05  9:13 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-22  7:52 [PATCH 00/21] media: atmel: atmel-isc: implement media controller Eugen Hristev
2021-10-22  7:52 ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 01/21] MAINTAINERS: add microchip csi2dc Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 02/21] dt-bindings: media: atmel: csi2dc: add bindings for " Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-29  1:57   ` Rob Herring
2021-10-29  1:57     ` Rob Herring
2021-10-22  7:52 ` [PATCH 03/21] media: atmel: introduce microchip csi2dc driver Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-11-02 17:22   ` Jacopo Mondi
2021-11-02 17:22     ` Jacopo Mondi
2021-11-03  8:31     ` Eugen.Hristev
2021-11-03  8:31       ` Eugen.Hristev
2021-11-03  9:28       ` Jacopo Mondi
2021-11-03  9:28         ` Jacopo Mondi
2021-11-03  9:59         ` Eugen.Hristev
2021-11-03  9:59           ` Eugen.Hristev
2021-11-03 10:59           ` Jacopo Mondi
2021-11-03 10:59             ` Jacopo Mondi
2021-11-11 10:05             ` Eugen.Hristev
2021-11-11 10:05               ` Eugen.Hristev
2021-10-22  7:52 ` [PATCH 04/21] MAINTAINERS: atmel-isc: add new file atmel-isc-clk.c Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 05/21] media: atmel: atmel-isc: split the clock code into separate source file Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-11-05  9:02   ` Jacopo Mondi
2021-11-05  9:02     ` Jacopo Mondi
2021-11-05  9:13     ` Eugen.Hristev [this message]
2021-11-05  9:13       ` Eugen.Hristev
2021-10-22  7:52 ` [PATCH 06/21] media: atmel: atmel-isc: replace video device name with module name Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-11-05  9:14   ` Jacopo Mondi
2021-11-05  9:14     ` Jacopo Mondi
2021-10-22  7:52 ` [PATCH 07/21] media: atmel: atmel-sama7g5-isc: fix ispck leftover Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-11-05  9:15   ` Jacopo Mondi
2021-11-05  9:15     ` Jacopo Mondi
2021-10-22  7:52 ` [PATCH 08/21] media: atmel: atmel-isc-base: use streaming status when queueing buffers Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-11-05  9:18   ` Jacopo Mondi
2021-11-05  9:18     ` Jacopo Mondi
2021-10-22  7:52 ` [PATCH 09/21] media: atmel: atmel-isc-base: remove frameintervals VIDIOC Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-11-05  9:23   ` Jacopo Mondi
2021-11-05  9:23     ` Jacopo Mondi
2021-10-22  7:52 ` [PATCH 10/21] media: atmel: atmel-isc-base: report frame sizes as full supported range Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-11-05  9:52   ` Jacopo Mondi
2021-11-05  9:52     ` Jacopo Mondi
2021-10-22  7:52 ` [PATCH 11/21] media: atmel: atmel-isc-base: implement mbus_code support in enumfmt Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-11-05  9:25   ` Jacopo Mondi
2021-11-05  9:25     ` Jacopo Mondi
2021-11-05  9:51     ` Jacopo Mondi
2021-11-05  9:51       ` Jacopo Mondi
2021-11-05 11:03       ` Eugen.Hristev
2021-11-05 11:03         ` Eugen.Hristev
2021-11-08 11:20         ` Jacopo Mondi
2021-11-08 11:20           ` Jacopo Mondi
2021-11-08 14:08           ` Eugen.Hristev
2021-11-08 14:08             ` Eugen.Hristev
2021-11-11  9:49             ` Eugen.Hristev
2021-11-11  9:49               ` Eugen.Hristev
2021-10-22  7:52 ` [PATCH 12/21] media: atmel: atmel-isc-base: fix bytesperline value for planar formats Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 13/21] MAINTAINERS: atmel-isc: add new file atmel-isc-mc.c Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 14/21] media: atmel: atmel-isc: implement media controller Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 15/21] ARM: dts: at91: sama7g5: add nodes for video capture Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 16/21] ARM: configs: at91: sama7: add xisc and csi2dc Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 17/21] ARM: multi_v7_defconfig: add atmel video pipeline modules Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 18/21] media: atmel: atmel-sama5d2-isc: fix wrong mask in YUYV format check Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 19/21] media: atmel: atmel-isc-base: use mutex to lock awb workqueue from streaming Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 20/21] media: atmel: atmel-isc-base: add wb debug messages Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev
2021-10-22  7:52 ` [PATCH 21/21] media: atmel: atmel-isc-base: clamp wb gain coefficients Eugen Hristev
2021-10-22  7:52   ` Eugen Hristev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ab248578-cbd3-7ed4-2b5d-28d56048303b@microchip.com \
    --to=eugen.hristev@microchip.com \
    --cc=Nicolas.Ferre@microchip.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jacopo@jmondi.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=sakari.ailus@iki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.