linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
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

      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).