Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <ba6366af-ecae-5f8b-f48b-6f93fbe833c0@arm.com>
References: <20171102065626.21835-1-chunyan.zhang@spreadtrum.com>
 <20171102065626.21835-6-chunyan.zhang@spreadtrum.com> <532aed43-03bf-ea26-bdc2-d7deb1093f53@arm.com>
 <ba6366af-ecae-5f8b-f48b-6f93fbe833c0@arm.com>
From:   Chunyan Zhang <zhang.lyra@gmail.com>
Date:   Fri, 3 Nov 2017 20:12:42 +0800
Message-ID: <CAAfSe-tt==o2ZDq=C=pa_oAsA-xG1XMO=Nvd5qbHRTVgPaLBsw@mail.gmail.com>
Subject: Re: [PATCH V3 05/11] clk: sprd: add mux clock support
To:     Julien Thierry <julien.thierry@arm.com>
Cc:     Chunyan Zhang <chunyan.zhang@spreadtrum.com>,
        Stephen Boyd <sboyd@codeaurora.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        linux-clk <linux-clk@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        Arnd Bergmann <arnd@arndb.de>, Mark Brown <broonie@kernel.org>,
        Xiaolong Zhang <xiaolong.zhang@spreadtrum.com>,
        Ben Li <ben.li@spreadtrum.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Orson Zhai <orson.zhai@spreadtrum.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 3 November 2017 at 02:22, Julien Thierry <julien.thierry@arm.com> wrote:
>
>
> On 02/11/17 18:11, Julien Thierry wrote:
>>
>> Hi,
>>
>> On 02/11/17 06:56, Chunyan Zhang wrote:
>>>
>>> This patch adds clock multiplexor support for Spreadtrum platforms,
>>> the mux clocks also can be found in sprd composite clocks, so
>>> provides two helpers that can be reused later on.
>>>
>>> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>>> ---
>>>   drivers/clk/sprd/Makefile |  1 +
>>>   drivers/clk/sprd/mux.c    | 89
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>   drivers/clk/sprd/mux.h    | 65 ++++++++++++++++++++++++++++++++++
>>>   3 files changed, 155 insertions(+)
>>>   create mode 100644 drivers/clk/sprd/mux.c
>>>   create mode 100644 drivers/clk/sprd/mux.h
>>>
>>> diff --git a/drivers/clk/sprd/Makefile b/drivers/clk/sprd/Makefile
>>> index 8cd5592..cee36b5 100644
>>> --- a/drivers/clk/sprd/Makefile
>>> +++ b/drivers/clk/sprd/Makefile
>>> @@ -2,3 +2,4 @@ obj-$(CONFIG_SPRD_COMMON_CLK)    += clk-sprd.o
>>>   clk-sprd-y    += common.o
>>>   clk-sprd-y    += gate.o
>>> +clk-sprd-y    += mux.o
>>> diff --git a/drivers/clk/sprd/mux.c b/drivers/clk/sprd/mux.c
>>> new file mode 100644
>>> index 0000000..5a344e0
>>> --- /dev/null
>>> +++ b/drivers/clk/sprd/mux.c
>>> @@ -0,0 +1,89 @@
>>> +/*
>>> + * Spreadtrum multiplexer clock driver
>>> + *
>>> + * Copyright (C) 2017 Spreadtrum, Inc.
>>> + * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/regmap.h>
>>> +
>>> +#include "mux.h"
>>> +
>>> +DEFINE_SPINLOCK(sprd_mux_lock);
>>> +EXPORT_SYMBOL_GPL(sprd_mux_lock);
>>> +
>>> +u8 sprd_mux_helper_get_parent(const struct sprd_clk_common *common,
>>> +                  const struct sprd_mux_internal *mux)
>>> +{
>>> +    unsigned int reg;
>>> +    u8 parent;
>>> +    int num_parents;
>>> +    int i;
>>> +
>>> +    sprd_regmap_read(common->regmap, common->reg, &reg);
>>> +    parent = reg >> mux->shift;
>>> +    parent &= (1 << mux->width) - 1;
>>> +
>>> +    if (mux->table) {
>>> +        num_parents = clk_hw_get_num_parents(&common->hw);
>>> +
>>> +        for (i = 0; i < num_parents; i++)
>>> +            if (parent == mux->table[i] ||
>>> +                (i < (num_parents - 1) && parent > mux->table[i] &&
>>> +                 parent < mux->table[i + 1]))
>>> +                return i;
>>> +        if (i == num_parents)
>>> +            return i - 1;
>>
>>
>> The if branch is not necessary since you only get there when the loop has
>> finished, so the condition is always true. And the loop can be simplified
>> to:
>>
>> for (i = 0; i < num_parents - 1; i++)
>>      if (parent >= mux->table[i] &&  parent < mux->table[i + 1])
>>          return i;
>>
>> return num_parents;
>
>
> Oops, meant to say "return num_parents - 1;" on that last line.

Yes, that makes the code nicer, and thanks to your reminder, I even
make it more clean by moving the check for 'mux->table' up.


Thanks,
Chunyan

>
> --
> Julien Thierry

From 1583047009469513458@xxx Fri Nov 03 12:14:34 +0000 2017
X-GM-THRID: 1582936966448450735
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread