All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Tomasz Figa <t.figa@samsung.com>
Cc: mark.rutland@arm.com, heiko@sntech.de, tomasz.figa@gmail.com,
	buserror@gmail.com, jacmet@sunsite.dk, augulis.darius@gmail.com,
	christer@weinigel.se, sylvester.nawrocki@gmail.com,
	m.szyprowski@samsung.com, kgene.kim@samsung.com,
	linux@arm.linux.org.uk, Samuel Ortiz <sameo@linux.intel.com>,
	kwangwoo.lee@gmail.com, mcuelenaere@gmail.com,
	devicetree-discuss@lists.ozlabs.org,
	linux-samsung-soc@vger.kernel.org, john.stultz@linaro.org,
	ghcstop@gmail.com, linux@simtec.co.uk,
	linux-arm-kernel@lists.infradead.org,
	broonie@opensource.wolfsonmicro.com, jekhor@gmail.com,
	kyungmin.park@samsung.com, tglx@linutronix.de
Subject: Re: [PATCH v4 04/14] mfd: Add Samsung PWM/timer master driver
Date: Fri, 5 Apr 2013 19:05:24 +0200	[thread overview]
Message-ID: <201304051905.24644.arnd@arndb.de> (raw)
In-Reply-To: <5507947.WEhmSUi7vd@amdc1227>

On Friday 05 April 2013, Tomasz Figa wrote:
> On Friday 05 of April 2013 18:39:58 Samuel Ortiz wrote:
> > Hi Tomasz,
> > 
> > On Thu, Apr 04, 2013 at 06:37:01PM +0200, Tomasz Figa wrote:
> > > This patch adds master driver for PWM/timer block available on many
> > > Samsung SoCs providing clocksource and PWM output capabilities.
>
> > Why is that an MFD driver, and why aren't you using the PWM APIs for it ?
> > Also, you probably want to look at using the regmap APIs for your IO.
> 
> The case of Samsung PWM timers is rather complicated. It is a hardware block 
> that can be used at the same time to generate PWM signal and as a system 
> clocksource.
> 
> There was a discussion on how to solve the problem of sharing the hardware:
> http://thread.gmane.org/gmane.linux.kernel.samsung-soc/16480/focus=16500
> (see the linked post and replies to it).
> 

I don't think anyone ever suggested using a private API though. I think
it's ok if the driver lives in drivers/mfd when it doesn't fit anywhere
else easily, but you should really register it to the pwm subsystem to
expose that functionality, not export functions that can be used by
a pwm shim driver, which even seems to be missing from the series.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 04/14] mfd: Add Samsung PWM/timer master driver
Date: Fri, 5 Apr 2013 19:05:24 +0200	[thread overview]
Message-ID: <201304051905.24644.arnd@arndb.de> (raw)
In-Reply-To: <5507947.WEhmSUi7vd@amdc1227>

On Friday 05 April 2013, Tomasz Figa wrote:
> On Friday 05 of April 2013 18:39:58 Samuel Ortiz wrote:
> > Hi Tomasz,
> > 
> > On Thu, Apr 04, 2013 at 06:37:01PM +0200, Tomasz Figa wrote:
> > > This patch adds master driver for PWM/timer block available on many
> > > Samsung SoCs providing clocksource and PWM output capabilities.
>
> > Why is that an MFD driver, and why aren't you using the PWM APIs for it ?
> > Also, you probably want to look at using the regmap APIs for your IO.
> 
> The case of Samsung PWM timers is rather complicated. It is a hardware block 
> that can be used at the same time to generate PWM signal and as a system 
> clocksource.
> 
> There was a discussion on how to solve the problem of sharing the hardware:
> http://thread.gmane.org/gmane.linux.kernel.samsung-soc/16480/focus=16500
> (see the linked post and replies to it).
> 

I don't think anyone ever suggested using a private API though. I think
it's ok if the driver lives in drivers/mfd when it doesn't fit anywhere
else easily, but you should really register it to the pwm subsystem to
expose that functionality, not export functions that can be used by
a pwm shim driver, which even seems to be missing from the series.

	Arnd

  reply	other threads:[~2013-04-05 17:05 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-04 16:36 [PATCH v4 00/14] ARM: samsung-time: Prepare for multiplatform support Tomasz Figa
2013-04-04 16:36 ` Tomasz Figa
2013-04-04 16:36 ` [PATCH v4 01/14] ARM: SAMSUNG: Move samsung-time to drivers/clocksource Tomasz Figa
2013-04-04 16:36   ` Tomasz Figa
2013-04-04 16:36 ` [PATCH v4 02/14] clocksource: samsung-time: Drop useless defines from public header Tomasz Figa
2013-04-04 16:36   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 03/14] clocksource: samsung-time: Use local register definitions Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 04/14] mfd: Add Samsung PWM/timer master driver Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-05 16:39   ` Samuel Ortiz
2013-04-05 16:39     ` Samuel Ortiz
2013-04-05 16:53     ` Tomasz Figa
2013-04-05 16:53       ` Tomasz Figa
2013-04-05 17:05       ` Arnd Bergmann [this message]
2013-04-05 17:05         ` Arnd Bergmann
2013-04-05 17:35         ` Tomasz Figa
2013-04-05 17:35           ` Tomasz Figa
2013-04-05 19:54           ` Arnd Bergmann
2013-04-05 19:54             ` Arnd Bergmann
2013-04-05 22:24             ` Tomasz Figa
2013-04-05 22:24               ` Tomasz Figa
2013-04-08 16:53               ` Tomasz Figa
2013-04-08 16:53                 ` Tomasz Figa
2013-04-10 22:35                 ` Arnd Bergmann
2013-04-10 22:35                   ` Arnd Bergmann
2013-04-11 16:28                   ` Mark Brown
2013-04-11 16:28                     ` Mark Brown
2013-04-11 16:44                   ` Tomasz Figa
2013-04-11 16:44                     ` Tomasz Figa
2013-04-11 21:08                     ` Arnd Bergmann
2013-04-11 21:08                       ` Arnd Bergmann
2013-04-04 16:37 ` [PATCH v4 05/14] ARM: SAMSUNG: Unify base address definitions of timer block Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 06/14] ARM: SAMSUNG: Add new PWM platform device Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 07/14] ARM: SAMSUNG: Set PWM platform data Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 08/14] clocksource: samsung-time: Use Samsung PWM/timer master driver Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 09/14] clocksource: samsung-time: Use variant data to get SoC-specific bits Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 10/14] clocksource: samsung-time: Use master driver to configure dividers Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 11/14] clocksource: samsung-time: Use clk_prepare_enable Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 12/14] clocksource: samsung-time: Use master driver to control PWM channels Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 13/14] clocksource: samsung-time: Move IRQ mask/ack handling to the driver Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 16:37 ` [PATCH v4 14/14] ARM: SAMSUNG: Remove unused PWM timer IRQ chip code Tomasz Figa
2013-04-04 16:37   ` Tomasz Figa
2013-04-04 23:15 ` [PATCH v4 00/14] ARM: samsung-time: Prepare for multiplatform support Heiko Stübner
2013-04-04 23:15   ` Heiko Stübner
2013-04-05 10:33   ` Tomasz Figa
2013-04-05 10:33     ` Tomasz Figa
2013-04-05 22:57 ` Tomasz Figa
2013-04-05 22:57   ` Tomasz Figa

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=201304051905.24644.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=augulis.darius@gmail.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=buserror@gmail.com \
    --cc=christer@weinigel.se \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=ghcstop@gmail.com \
    --cc=heiko@sntech.de \
    --cc=jacmet@sunsite.dk \
    --cc=jekhor@gmail.com \
    --cc=john.stultz@linaro.org \
    --cc=kgene.kim@samsung.com \
    --cc=kwangwoo.lee@gmail.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=linux@simtec.co.uk \
    --cc=m.szyprowski@samsung.com \
    --cc=mark.rutland@arm.com \
    --cc=mcuelenaere@gmail.com \
    --cc=sameo@linux.intel.com \
    --cc=sylvester.nawrocki@gmail.com \
    --cc=t.figa@samsung.com \
    --cc=tglx@linutronix.de \
    --cc=tomasz.figa@gmail.com \
    /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.