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
References: <20190202155209.31617-1-plaes@plaes.org> <20190205094529.t7je4ozzu2b4ornc@flea>
In-Reply-To: <20190205094529.t7je4ozzu2b4ornc@flea>
From:   Chen-Yu Tsai <wens@csie.org>
Date:   Tue, 5 Feb 2019 21:44:02 +0800
Message-ID: <CAGb2v673ntXn7MTRhU1uecO9q8_i79e+Ovio=tucd6qFf5L84Q@mail.gmail.com>
Subject: Re: [RFC PATCH] clk: sunxi-ng: sun4i: Use CLK_SET_RATE_PARENT for
 mmc2 clock
To:     Maxime Ripard <maxime.ripard@bootlin.com>
Cc:     Priit Laes <plaes@plaes.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        linux-clk <linux-clk@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Jernej Skrabec <jernej.skrabec@siol.net>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Feb 5, 2019 at 5:45 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Sat, Feb 02, 2019 at 05:52:09PM +0200, Priit Laes wrote:
> > Recent patch of improving MP clock rate calculations by taking
> > into account whether adjusting parent rate is allowed, have
> > unfortunately broken eMMC support on A20 Olinuxino-Lime2-eMMC
> > boards which fail with following error:
> >
> > [snip]
> > EXT4-fs (mmcblk1p4): INFO: recovery required on readonly filesystem
> > EXT4-fs (mmcblk1p4): write access will be enabled during recovery
> > sunxi-mmc 1c11000.mmc: data error, sending stop command
> > sunxi-mmc 1c11000.mmc: send stop command failed
> > [/snip]
> >
> > Previously, mmc2 clock was requesting 520MHz and settling at 512MHz
> > clock rate with following parents:

You mean 52 and 51.2 MHz.

> > [snip]
> > pll-ddr-base        2  2        0   768000000 0     0  50000
> >    pll-ddr-other    1  1        0   768000000 0     0  50000
> >       mmc2          0  0        0    51200000 0     0  50000
> > [/snip]
> >
> > Now, after the improvements, requested and settled rate are both
> > 520MHz, but as mmc2 clock cannot adjust parent rate, the situation
> > ends up like this:
> > [snip]
> > pll-periph-base     3  3        0  1200000000 0     0  50000
> >    pll-periph       6  6        0   600000000 0     0  50000
> >       mmc2          3  3        0    50000000 0     0  50000
> > [/snip]
> >
> > With this patch (allowing mmc2 to set parent rate), we end up with
> > working tree with both mmc0 (sd-card) and mmc2 (eMMC) working:
> > [snip]
> > pll-periph-base     3  3        0   312000000 0     0  50000
> >    mbus             1  1        0    78000000 0     0  50000
> >    pll-periph-sata  1  1        0    26000000 0     0  50000
> >       sata          1  1        0    26000000 0     0  50000
> >    pll-periph       5  5        0   156000000 0     0  50000
> >       mmc2          0  0        0    52000000 0     0  50000
> >       mmc0          0  0        0    39000000 0     0  50000
> > [/snip]
> >
> > Fixes: 3f790433c3cb ("clk: sunxi-ng: Adjust MP clock parent rate when allowed")
> > Signed-off-by: Priit Laes <plaes@plaes.org>
>
> Applied, thanks!
> Maxime

I'm concerned for other users of the PLL-PERIPH clock. AFAIK
all of them, except the HRTIMER, expect the clock rate to stay
the same and not change underneath them. And SATA expects it to
be at 600 MHz, as the datasheet says. And while it may not directly
apply to the LIME2, eMMC on newer SoCs / boards run at the slightly
reduced rate of 50 MHz just fine.

In the commit in question, clocks without CLK_SET_RATE_PARENT
should be using the old code (now in the if conditional block),
i.e. the behavior should not have changed.

I don't think this actually "fixes" whatever bug was introduced,
but only papers over the issue, and possible introduces further
issues for other users.

Regards
ChenYu