From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752769AbdLKKov (ORCPT <rfc822;w@1wt.eu>);
        Mon, 11 Dec 2017 05:44:51 -0500
Received: from mail-lf0-f68.google.com ([209.85.215.68]:39663 "EHLO
        mail-lf0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752609AbdLKKoq (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 11 Dec 2017 05:44:46 -0500
X-Google-Smtp-Source: AGs4zMYaGI3Q26qil/jV/ErzN73kGbyYiSJnNCBVU9VxX8TZQHnEoyWNfO+pcInYhZyMoSWmIuiVM11jj2uo0oIDV4s=
MIME-Version: 1.0
In-Reply-To: <CAK8P3a10wjXvFpE5zbwCfQNC-UiRCzKE+9-ENg-Sr577ZCF+Xw@mail.gmail.com>
References: <20171211050704.20621-1-joel@jms.id.au> <20171211050704.20621-6-joel@jms.id.au>
 <CAK8P3a10wjXvFpE5zbwCfQNC-UiRCzKE+9-ENg-Sr577ZCF+Xw@mail.gmail.com>
From: Joel Stanley <joel@jms.id.au>
Date: Mon, 11 Dec 2017 21:14:23 +1030
X-Google-Sender-Auth: VyNmZoS2eqFAKiB2elVHIAg4Wfw
Message-ID: <CACPK8XeTtiejw1SyDHNn5w6be-FqCQFuT5PT-yeRho2Hhs2K8g@mail.gmail.com>
Subject: Re: [PATCH 05/20] ARM: dts: aspeed: Add proper clock references
To: Arnd Bergmann <arnd@arndb.de>, Andrew Jeffery <andrew@aj.id.au>
Cc: Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
        Patrick Venture <venture@google.com>, Xo Wang <xow@google.com>,
        Lei YU <mine260309@gmail.com>,
        =?UTF-8?Q?C=C3=A9dric_Le_Goater?= <clg@kaod.org>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Jeremy Kerr <jk@ozlabs.org>, DTML <devicetree@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-aspeed@lists.ozlabs.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Dec 11, 2017 at 6:39 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Dec 11, 2017 at 6:06 AM, Joel Stanley <joel@jms.id.au> wrote:
>> The existing device trees use fixed-clocks in order to boot without a
>> clk driver. The newly added clk driver provides proper clock support,
>> including gating, so we move the device trees over to properly request
>> clocks.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>
> Can you clarify here whether this will break running old kernels with
> new DT files or vice versa, and why this is ok here?

This device tree will break kernels that do not have the clk patches
applied (no clocksource, as we don't know the speed of the APB clock.
You can boot if you pass a lpj value on the command line, but won't
have a uart).

Older device trees running with the newer kernel will function as well
as pre-4.16 kernels. That is, that some IP blocks (i2c, pwm/tach, adc)
will not work as the kernel lacks reset controller and clock enabling.

> I assume you have thought about it carefully, but I'd still like to document
> every time we intentionally break compatibility like this. It looks like
> you too care to merge the driver changes and the DT binding change first,
> so we don't get any bisection problems.

Thanks for calling it out. I will modify the commit message to say
that this device tree change depends on the newer driver, and it will
not boot with kernels that lack the driver.

>
> What I'm not completely clear about is the difference between the
> "aspeed,g4-scu" binding and the "aspeed,ast2400-scu" binding.
> They are listed as equal in
> Documentation/devicetree/bindings/mfd/aspeed-scu.txt, so why do you
> change it here?

The g4-scu string made it into the tree before we had decided that we
would settle on aspeed,astX000-<ip> as the format for the strings. We
list both in the docs, but I would like to deprecate the old one.

If I was doing this again, I would make sure we had the clock driver
upstream before completing the other driver. It's caused a lot of
pain. Thanks for your help getting us there.

Cheers,

Joel

From mboxrd@z Thu Jan  1 00:00:00 1970
From: joel@jms.id.au (Joel Stanley)
Date: Mon, 11 Dec 2017 21:14:23 +1030
Subject: [PATCH 05/20] ARM: dts: aspeed: Add proper clock references
In-Reply-To: <CAK8P3a10wjXvFpE5zbwCfQNC-UiRCzKE+9-ENg-Sr577ZCF+Xw@mail.gmail.com>
References: <20171211050704.20621-1-joel@jms.id.au>
 <20171211050704.20621-6-joel@jms.id.au>
 <CAK8P3a10wjXvFpE5zbwCfQNC-UiRCzKE+9-ENg-Sr577ZCF+Xw@mail.gmail.com>
Message-ID: <CACPK8XeTtiejw1SyDHNn5w6be-FqCQFuT5PT-yeRho2Hhs2K8g@mail.gmail.com>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

On Mon, Dec 11, 2017 at 6:39 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Dec 11, 2017 at 6:06 AM, Joel Stanley <joel@jms.id.au> wrote:
>> The existing device trees use fixed-clocks in order to boot without a
>> clk driver. The newly added clk driver provides proper clock support,
>> including gating, so we move the device trees over to properly request
>> clocks.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>
> Can you clarify here whether this will break running old kernels with
> new DT files or vice versa, and why this is ok here?

This device tree will break kernels that do not have the clk patches
applied (no clocksource, as we don't know the speed of the APB clock.
You can boot if you pass a lpj value on the command line, but won't
have a uart).

Older device trees running with the newer kernel will function as well
as pre-4.16 kernels. That is, that some IP blocks (i2c, pwm/tach, adc)
will not work as the kernel lacks reset controller and clock enabling.

> I assume you have thought about it carefully, but I'd still like to document
> every time we intentionally break compatibility like this. It looks like
> you too care to merge the driver changes and the DT binding change first,
> so we don't get any bisection problems.

Thanks for calling it out. I will modify the commit message to say
that this device tree change depends on the newer driver, and it will
not boot with kernels that lack the driver.

>
> What I'm not completely clear about is the difference between the
> "aspeed,g4-scu" binding and the "aspeed,ast2400-scu" binding.
> They are listed as equal in
> Documentation/devicetree/bindings/mfd/aspeed-scu.txt, so why do you
> change it here?

The g4-scu string made it into the tree before we had decided that we
would settle on aspeed,astX000-<ip> as the format for the strings. We
list both in the docs, but I would like to deprecate the old one.

If I was doing this again, I would make sure we had the clock driver
upstream before completing the other driver. It's caused a lot of
pain. Thanks for your help getting us there.

Cheers,

Joel