public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [GIT PULL] Broadcom SoC changes for 4.2
Date: Thu, 21 May 2015 00:02:48 +0200	[thread overview]
Message-ID: <3733749.guFogFhQqU@wuerfel> (raw)
In-Reply-To: <555D01AD.2040705@broadcom.com>

On Wednesday 20 May 2015 14:50:37 Florian Fainelli wrote:
> >>> - it is in include/linux/ where it clearly does not belong, as no other component
> >>>   should be including it. Even the function documentation in there mentions that
> >>>   one must hold the pmb_lock before calling it, and that is defined statically
> >>>   arch/arm/mach-bcm/bcm63xx_pmb.c, so it's impossible for other code to use.
> >>>   Just move it all into bcm63xx_pmb.c.
> >>
> >> This header will later be used by the bcm63138 reset controller, and I
> >> thought I had explained the reasoning behind that in either the commit
> >> message or cover letter, I will make sure the commit message explains it.
> > 
> > I see. I still think it's a bit rude to place this header in the top-level
> > include/linux directory though. I realize there are a lot of other headers
> > like this, but I'm trying not to add too many more.
> 
> Would a lesser evil be to create include/linux/resets/ and place this
> header file there?

I've also thought about that. It would certainly help.

> > Maybe a lesser evil would be to put the reset driver into
> > arch/arm/mach-bcm/bcm63xx_pmb.c as well?
> > 
> > How big is it? And is there anything else besides that driver which would
> > need these functions?
> 
> It is going to be (once feature complete) as big as
> arch/arm/mach-bcm/bcm63xx_pmb.c except that it will also have to inspect
> the client asking for the reset, e.g: the reset procedure for the SATA
> block is a little different than the one for the PCIe PHY, or integrated
> Ethernet switch, or USB controlers...
> 
> The way we power on a secondary CPU is code that is not shared with how
> other on-chip peripherals are powered on, hence the idea behind the
> separation.

Ok, I see. Let's start with the include/linux/resets/ approach then.

Yet another idea would be to expose the read/write interface here
as a regmap and find a way to share that. That way, you could move
a large part of bcm63xx_pmb.c and bcm63xx_pmb.h into the reset
driver and just have one interface to get the regmap, like we do
for syscon devices. It would still need a single function declaration
though.

	Arnd

  reply	other threads:[~2015-05-20 22:02 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-13 22:34 [GIT PULL] Broadcom Device Tree changes for 4.2 Florian Fainelli
2015-05-13 22:34 ` [GIT PULL] Broadcom MAINTAINERS file update " Florian Fainelli
2015-05-20 15:44   ` Arnd Bergmann
2015-05-13 22:34 ` [GIT PULL] Broadcom SoC changes " Florian Fainelli
2015-05-20 15:42   ` Arnd Bergmann
2015-05-20 15:43   ` Arnd Bergmann
2015-05-20 20:59   ` Arnd Bergmann
2015-05-20 21:05     ` Florian Fainelli
2015-05-20 21:22       ` Arnd Bergmann
2015-05-20 21:50         ` Florian Fainelli
2015-05-20 22:02           ` Arnd Bergmann [this message]
2015-05-15 15:18 ` [GIT PULL] Broadcom Device Tree " Arnd Bergmann

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=3733749.guFogFhQqU@wuerfel \
    --to=arnd@arndb.de \
    --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