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;
Subject: Re: [PATCH v1 1/2] clk: intel: Add CGU clock driver for a new SoC
To:     Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc:     andriy.shevchenko@intel.com, cheol.yong.kim@intel.com,
        linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
        mark.rutland@arm.com, mturquette@baylibre.com,
        qi-ming.wu@intel.com, rahul.tanwar@intel.com, robh+dt@kernel.org,
        robh@kernel.org, sboyd@kernel.org, yixin.zhu@linux.intel.com
References: <6a3c26bc6e25d883686287883528dbde30725922.1566975410.git.rahul.tanwar@linux.intel.com>
 <20190902222015.11360-1-martin.blumenstingl@googlemail.com>
 <d9e96dab-96be-0c14-b7af-e1f2dc07ebd2@linux.intel.com>
 <CAFBinCARQJ7q9q3r6c6Yr2SD0Oo_Drah-kxss3Obs-g=B1M28A@mail.gmail.com>
From:   "Tanwar, Rahul" <rahul.tanwar@linux.intel.com>
Message-ID: <b7920723-1df2-62df-61c7-98c3a1665aa1@linux.intel.com>
Date:   Wed, 4 Sep 2019 16:03:59 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <CAFBinCARQJ7q9q3r6c6Yr2SD0Oo_Drah-kxss3Obs-g=B1M28A@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Hi Martin,

On 4/9/2019 2:53 AM, Martin Blumenstingl wrote:
>> My understanding is that if we do not use syscon, then there is no
>> point in using regmap because this driver uses simple 32 bit register
>> access. Can directly read/write registers using readl() & writel().
>>
>> Would you agree ?
> if there was only the LGM SoC then I would say: drop regmap
>
> however, last year a driver for the GRX350/GRX550 SoCs was proposed: [0]
> this was never updated but it seems to use the same "framework" as the
> LGM driver
> with this in mind I am for keeping regmap support because.
> I think it will be easier to add support for old SoCs like
> GRX350/GRX550 (but also VRX200), because the PLL sub-driver (I am
> assuming that it is similar on all SoCs) or some other helpers can be
> re-used across various SoCs instead of "duplicating" code (where one
> variant would use regmap and the other readl/writel).


Earlier, we had discussed about it in our team.  There are no plans to

upstream mips based platform code, past up-streaming efforts for mips

platforms were also dropped. GRX350/GRX550/VRX200 are all mips

based platforms. Plan is to upstream only x86 based platforms. In-fact,

i had removed GRX & other older SoCs support from this driver before

sending for review. So we can consider only x86 based LGM family of

SoCs for this driver & all of them will be reusing same IP.

> [...]
>>> +     select OF_EARLY_FLATTREE
>>> there's not a single other "select OF_EARLY_FLATTREE" in driver/clk
>>> I'm not saying this is wrong but it makes me curious why you need this
>>
>> We need OF_EARLY_FLATTREE for LGM. But adding a new x86
>> platform for LGM is discouraged because that would lead to too
>> many platforms. Only differentiating factor for LGM is CPU model
>> ID but it can differentiate only at run time. So i had no option
>> other then enabling it with some LGM specific core system module
>> driver and CGU seemed perfect for this purpose.
> so when my x86 kernel maintainer enables CONFIG_INTEL_LGM_CGU_CLK then
> OF_EARLY_FLATTREE is enabled as well.
> does this hurt any existing x86 platform? if not: why can't we enable
> it for x86 unconditionally?


IMHO, it will not hurt any other existing x86 platform but enabling it for

x86 unconditionally also doesn't sound like a good idea. I now get your

point that enabling OF_EARLY_FLATTREE here is a bit odd. I will remove

it in next patch.

Regards,

Rahul

> I went through meson & qcom regmap clock code. Agree, it can be
> reused for mux, divider and gate. But as mentioned above, i am now
> considering to move away from using regmap.
> thank you for evaluating them. let's continue the discussion above
> whether regmap should be used - after that we decide (if needed) which
> regmap implementation to use
>
>
> Martin
>
>
> [0] https://patchwork.kernel.org/patch/10554401/