From: Jisheng.Zhang@synaptics.com (Jisheng Zhang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] mmc: sdhci-xenon: Fix clock resource by adding an optional bus clock
Date: Sat, 30 Sep 2017 10:56:38 +0800 [thread overview]
Message-ID: <20170930105638.145dcb1e@xhacker.debian> (raw)
In-Reply-To: <CAG18LKhU0Ecjmh=7_YAM8pOpO295Fu1NXy2mL46KwxkVmQ9O2A@mail.gmail.com>
Hi Ziji,
On Sat, 30 Sep 2017 10:47:39 +0800 Ziji Hu wrote:
> Hi Gregory,
>
> 2017-09-28 23:05 GMT+08:00 Gregory CLEMENT <gregory.clement@free-electrons.com>:
> > On Armada 7K/8K we need to explicitly enable the bus clock. The bus clock
> > is optional because not all the SoCs need them but at least for Armada
> > 7K/8K it is actually mandatory.
> >
> > The binding documentation is updating accordingly.
> >
> > Without this patch the kernel hand during boot if the mvpp2.2 network
> > driver was not present in the kernel. Indeed the clock needed by the
> > xenon controller was set by the network driver.
> >
> > Fixes: 3a3748dba881 ("mmc: sdhci-xenon: Add Marvell Xenon SDHC core
> > functionality)"
> > CC: Stable <stable@vger.kernel.org>
> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > ---
> > Hi Ulf and Adrian,
> >
> > This patch should be merged in 4.14-rc, as it fixes real issues.
> >
> > This patch maye looks like just as a nice clean-up but it is not. On
> > the earlier version of the series adding the support for the xenon
> > controller there was already this axi bus clock. But at this moment
> > the documentation for the clock on the Armada 7K/8K was missing and
> > there was no driver for it, so we just use the clock set by the
> > bootloader and never change them that's why it worked without any
> > visible problem.
> >
> > Then as explained in the commit log the network driver enabled the
> > clock for us, and it happened that it was setup before the xenon
> > driver. It is only thanks the new information we received on the
> > clocks of the Sov and with more exhaustive testes that we found this
> > issue and the fix for it.
>
> I know very little about clk in Linux.
> But I just happened to review this very issue before I left Marvell.
> I had some concerns about this patch at that time.
> Please check the following.
>
> 1. If that network device is used again, so that clock will be setup *twice*
> since there are two duplicated clock setup in that network driver
> and Xenon driver. Right?
The first "setup" will be executed, the second one just increase the enable
and prepare count.
> 2. If that network device is used again and Xenon later exists and
> shutdown the clock,
> will that network device be corrupted?
No. The enable and prepare count is reduced to 1, nothing else could happen.
The clock won't be shutdown until there's no users.
> 3. In my very own opinion, such a shared AXI bus clock might be
> controlled in u-boot or
> a "SoC-level" init routine.
IMHO, it's the driver's responsibility to manage its own clocks.
Thanks
prev parent reply other threads:[~2017-09-30 2:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-28 15:05 [PATCH] mmc: sdhci-xenon: Fix clock resource by adding an optional bus clock Gregory CLEMENT
2017-09-28 15:57 ` Gregory CLEMENT
2017-09-29 2:39 ` Jisheng Zhang
2017-09-29 9:13 ` Gregory CLEMENT
2017-09-29 9:15 ` Thomas Petazzoni
2017-09-29 9:22 ` Gregory CLEMENT
2017-09-30 2:47 ` Ziji Hu
2017-09-30 2:56 ` Jisheng Zhang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170930105638.145dcb1e@xhacker.debian \
--to=jisheng.zhang@synaptics.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).