Re: [PATCH v2 14/18] serial: intel: Add CCF support

From: Arnd Bergmann <arnd@arndb.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Hauke Mehrtens <hauke@hauke-m.de>,
	"Wu, Songjun" <songjun.wu@linux.intel.com>,
	hua.ma@linux.intel.com, yixin.zhu@linux.intel.com,
	chuanhua.lei@linux.intel.com, qi-ming.wu@intel.com,
	"open list:RALINK MIPS ARCHITECTURE" <linux-mips@linux-mips.org>,
	linux-clk <linux-clk@vger.kernel.org>,
	linux-serial@vger.kernel.org, DTML <devicetree@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Jiri Slaby <jslaby@suse.com>
Subject: Re: [PATCH v2 14/18] serial: intel: Add CCF support
Date: Sat, 4 Aug 2018 23:03:11 +0200	[thread overview]
Message-ID: <CAK8P3a3qs34LuhPeaef2wPHYEWbYO5N-4n7763BcaDyppiJ6DA@mail.gmail.com> (raw)
In-Reply-To: <20180804124309.GB4920@kroah.com>

On Sat, Aug 4, 2018 at 2:43 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Sat, Aug 04, 2018 at 12:54:22PM +0200, Hauke Mehrtens wrote:
>> On 08/03/2018 12:30 PM, Greg Kroah-Hartman wrote:
>> > On Fri, Aug 03, 2018 at 03:33:38PM +0800, Wu, Songjun wrote:

>> This patch makes it possible to use it with the legacy lantiq code and
>> also with the common clock framework. I see multiple options to fix this
>> problem.
>>
>> 1. The current approach to have it as a compile variant for a) legacy
>> lantiq arch code without common clock framework and b) support for SoCs
>> using the common clock framework.
>> 2. Convert the lantiq arch code to the common clock framework. This
>> would be a good approach, but it need some efforts.
>> 3. Remove the arch/mips/lantiq code. There are still users of this code.
>> 4. Use the old APIs also for the new xRX500 SoC, I do not like this
>> approach.
>> 5. Move lantiq_soc.h to somewhere in include/linux/ so it is globally
>> available and provide some better wrapper code.
>
> I don't really care what you do at this point in time, but you all
> should know better than the crazy #ifdef is not allowed to try to
> prevent/allow the inclusion of a .h file.  Checkpatch might have even
> warned you about it, right?
>
> So do it correctly, odds are #5 is correct, as that makes it work like
> any other device in the kernel.  You are not unique here.

The best approach here would clearly be 2. We don't want platform
specific header files for doing things that should be completely generic.

Converting lantiq to the common-clk framework obviously requires
some work, but then again the whole arch/mips/lantiq/clk.c file
is fairly short and maybe not that hard to convert.

From looking at arch/mips/lantiq/xway/sysctrl.c, it appears that you
already use the clkdev lookup mechanism for some devices without
using COMMON_CLK, so I would assume that you can also use those
for the remaining clks, which would be much simpler. It registers
one anonymous clk there as

        clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);

so why not add replace that with two named clocks and just use
the same names in the DT for the newer chip?

      Arnd