All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Wahren <stefan.wahren@i2se.com>
To: Eric Anholt <eric@anholt.net>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	Guenter Roeck <linux@roeck-us.net>,
	linux-watchdog@vger.kernel.org
Cc: linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	bcm-kernel-feedback-list@broadcom.com
Subject: Re: [PATCH 0/8] BCM2835 PM driver
Date: Tue, 20 Nov 2018 18:49:31 +0100 (CET)	[thread overview]
Message-ID: <1299453058.112996.1542736171394@email.ionos.de> (raw)
In-Reply-To: <20181120172000.15102-1-eric@anholt.net>

Hi Eric,

> Eric Anholt <eric@anholt.net> hat am 20. November 2018 um 18:19 geschrieben:
> 
> 
> This series moves the BCM2835 WDT driver that controls a fraction of
> the PM block out to soc/ and adds most of the rest of its
> functionality.  My motivation has been to have V3D be functional
> without firmware calls, probably improve its interactivity (since
> we'll be able to power on/off without RPC to the firmware that may be
> busy with other tasks), and (in a patch not submitted in this series)
> extend its binding to use the reset controller instead of trying to
> reset by toggling its power domain.
> 
> I've tested V3D with a few hours of running a V3D test, sleep(1) (to
> trigger PM domain off); running a GPU hang job (to trigger reset);
> sleep(1).  The non-hanging success-case job always passed, and dmesg
> had no complaints from bcm2835-pm.  The other power domains are not
> tested, but I've done my best.
> 
> This series will probably also be of interest to the
> https://github.com/christinaa/rpi-open-firmware project for enabling USB.
> 

apologize to give you my feedback after you send out the series.

I know you won't be happy about it, but i think we need a little more complex but future proof solution for this power driver. According to the register definition of the PM block, we have multiple functions here (power domains, watchdog, pads/pinctrl, ...). Since this is common for ARM SoCs there is a subsystem called mfd (multi function device) [1] to abstract all resources of the IP block.

This has the advantage that we don't need a monolithic driver which takes care of all functions.

According to this approach we would have the following drivers:
mfd/bcm2835.c
soc/bcm/bcm2835-power.c
watchdog/bcm2835_wdt.c

Best regards
Stefan

[1] - http://events17.linuxfoundation.org/sites/events/files/slides/belloni-mfd-regmap-syscon_0.pdf

WARNING: multiple messages have this Message-ID (diff)
From: stefan.wahren@i2se.com (Stefan Wahren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 0/8] BCM2835 PM driver
Date: Tue, 20 Nov 2018 18:49:31 +0100 (CET)	[thread overview]
Message-ID: <1299453058.112996.1542736171394@email.ionos.de> (raw)
In-Reply-To: <20181120172000.15102-1-eric@anholt.net>

Hi Eric,

> Eric Anholt <eric@anholt.net> hat am 20. November 2018 um 18:19 geschrieben:
> 
> 
> This series moves the BCM2835 WDT driver that controls a fraction of
> the PM block out to soc/ and adds most of the rest of its
> functionality.  My motivation has been to have V3D be functional
> without firmware calls, probably improve its interactivity (since
> we'll be able to power on/off without RPC to the firmware that may be
> busy with other tasks), and (in a patch not submitted in this series)
> extend its binding to use the reset controller instead of trying to
> reset by toggling its power domain.
> 
> I've tested V3D with a few hours of running a V3D test, sleep(1) (to
> trigger PM domain off); running a GPU hang job (to trigger reset);
> sleep(1).  The non-hanging success-case job always passed, and dmesg
> had no complaints from bcm2835-pm.  The other power domains are not
> tested, but I've done my best.
> 
> This series will probably also be of interest to the
> https://github.com/christinaa/rpi-open-firmware project for enabling USB.
> 

apologize to give you my feedback after you send out the series.

I know you won't be happy about it, but i think we need a little more complex but future proof solution for this power driver. According to the register definition of the PM block, we have multiple functions here (power domains, watchdog, pads/pinctrl, ...). Since this is common for ARM SoCs there is a subsystem called mfd (multi function device) [1] to abstract all resources of the IP block.

This has the advantage that we don't need a monolithic driver which takes care of all functions.

According to this approach we would have the following drivers:
mfd/bcm2835.c
soc/bcm/bcm2835-power.c
watchdog/bcm2835_wdt.c

Best regards
Stefan

[1] - http://events17.linuxfoundation.org/sites/events/files/slides/belloni-mfd-regmap-syscon_0.pdf

  parent reply	other threads:[~2018-11-20 17:50 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-20 17:19 [PATCH 0/8] BCM2835 PM driver Eric Anholt
2018-11-20 17:19 ` Eric Anholt
2018-11-20 17:19 ` [PATCH 1/8] watchdog: bcm2835: Move the driver to the soc/ directory Eric Anholt
2018-11-20 17:19   ` Eric Anholt
2018-11-20 17:29   ` Guenter Roeck
2018-11-20 17:29     ` Guenter Roeck
2018-11-20 17:19 ` [PATCH 2/8] soc: bcm: bcm2835-pm: Rename the driver to its new "PM" name Eric Anholt
2018-11-20 17:19   ` Eric Anholt
2018-11-20 17:19 ` [PATCH 3/8] soc: bcm: bcm2835-pm: Stop using _relaxed mmio accessors Eric Anholt
2018-11-20 17:19   ` Eric Anholt
2018-11-20 17:19 ` [PATCH 4/8] soc: bcm: bcm2835-pm: Make some little accessor macros for the mmio area Eric Anholt
2018-11-20 17:19   ` Eric Anholt
2018-11-20 17:19 ` [PATCH 5/8] dt-bindings: soc: Add a new binding for the BCM2835 PM node Eric Anholt
2018-11-20 17:19   ` Eric Anholt
2018-11-20 17:19 ` [PATCH 6/8] soc: bcm: bcm2835-pm: Add support for power domains under a new binding Eric Anholt
2018-11-20 17:19   ` Eric Anholt
2018-11-20 17:19 ` [PATCH 7/8] ARM: bcm283x: Extend the WDT DT node out to cover the whole PM block Eric Anholt
2018-11-20 17:19   ` Eric Anholt
2018-11-20 17:20 ` [PATCH 8/8] ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware Eric Anholt
2018-11-20 17:20   ` Eric Anholt
2018-11-20 17:49 ` Stefan Wahren [this message]
2018-11-20 17:49   ` [PATCH 0/8] BCM2835 PM driver Stefan Wahren
2018-11-20 21:34   ` Eric Anholt
2018-11-20 21:34     ` Eric Anholt
2018-11-20 21:39     ` Arnd Bergmann
2018-11-20 21:39       ` Arnd Bergmann
2018-11-20 21:39       ` 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=1299453058.112996.1542736171394@email.ionos.de \
    --to=stefan.wahren@i2se.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=eric@anholt.net \
    --cc=f.fainelli@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=wim@linux-watchdog.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.