From: Andrew Morton <akpm@linux-foundation.org>
To: Wolfram Sang <w.sang@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
Shawn Guo <shawn.guo@linaro.org>,
Huang Shijie <b32955@freescale.com>
Subject: Re: [PATCH V2 1/3] lib: add support for stmp-style devices
Date: Sun, 22 Apr 2012 23:55:06 -0700 [thread overview]
Message-ID: <20120422235506.d3377f0e.akpm@linux-foundation.org> (raw)
In-Reply-To: <1332368519-22660-2-git-send-email-w.sang@pengutronix.de>
On Wed, 21 Mar 2012 23:21:57 +0100 Wolfram Sang <w.sang@pengutronix.de> wrote:
> MX23/28 use IP cores which follow a register layout I have first seen on
> STMP3xxx SoCs. In this layout, every register actually has four u32:
>
> 1.) to store a value directly
> 2.) a SET register where every 1-bit sets the corresponding bit,
> others are unaffected
> 3.) same with a CLR register
> 4.) same with a TOG (toggle) register
>
> Also, the 2 MSBs in register 0 are always the same and can be used to reset
> the IP core.
>
> All this is strictly speaking not mach-specific (but IP core specific) and,
> thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
> utilize IP cores following this stmp-style. So:
>
> Introduce a stmp-style device, put the code and defines for that in a public
> place (lib/), and let drivers for stmp-style devices select that code.
> To avoid regressions and ease reviewing, the actual code is simply copied from
> mach-mxs. It definately wants updates, but those need a seperate patch series.
>
> Voila, mach dependency gone, reusable code introduced. Note that I didn't
> remove the duplicated code from mach-mxs yet, first the drivers have to be
> converted.
>
> ...
>
> include/linux/stmp_device.h | 20 +++++++++++
> lib/Kconfig | 3 ++
> lib/Makefile | 2 +
> lib/stmp_device.c | 80 +++++++++++++++++++++++++++++++++++++++++++
It's good that this is being presented as library code, rather than
being buried in some device-specific directory then copied and pasted
ten times.
But ./lib/ does seem rather a strange place for it. Perhaps we need a
drivers/lib/ or something. We can use ./lib/ for now - it can always
be moved later on.
> --- /dev/null
> +++ b/lib/stmp_device.c
The functions in this file look terribly racy on SMP, or even with
preemption or interrupts. What happens if two CPUs or threads run
stmp_reset_block() against the same device at the same time?
Perhaps the caller is supposed to prevent that, and the documentation
which isn't there forgot to mention it ;)
WARNING: multiple messages have this Message-ID (diff)
From: akpm@linux-foundation.org (Andrew Morton)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2 1/3] lib: add support for stmp-style devices
Date: Sun, 22 Apr 2012 23:55:06 -0700 [thread overview]
Message-ID: <20120422235506.d3377f0e.akpm@linux-foundation.org> (raw)
In-Reply-To: <1332368519-22660-2-git-send-email-w.sang@pengutronix.de>
On Wed, 21 Mar 2012 23:21:57 +0100 Wolfram Sang <w.sang@pengutronix.de> wrote:
> MX23/28 use IP cores which follow a register layout I have first seen on
> STMP3xxx SoCs. In this layout, every register actually has four u32:
>
> 1.) to store a value directly
> 2.) a SET register where every 1-bit sets the corresponding bit,
> others are unaffected
> 3.) same with a CLR register
> 4.) same with a TOG (toggle) register
>
> Also, the 2 MSBs in register 0 are always the same and can be used to reset
> the IP core.
>
> All this is strictly speaking not mach-specific (but IP core specific) and,
> thus, doesn't need to be in mach-mxs/include. At least, mx6 and mx50 also
> utilize IP cores following this stmp-style. So:
>
> Introduce a stmp-style device, put the code and defines for that in a public
> place (lib/), and let drivers for stmp-style devices select that code.
> To avoid regressions and ease reviewing, the actual code is simply copied from
> mach-mxs. It definately wants updates, but those need a seperate patch series.
>
> Voila, mach dependency gone, reusable code introduced. Note that I didn't
> remove the duplicated code from mach-mxs yet, first the drivers have to be
> converted.
>
> ...
>
> include/linux/stmp_device.h | 20 +++++++++++
> lib/Kconfig | 3 ++
> lib/Makefile | 2 +
> lib/stmp_device.c | 80 +++++++++++++++++++++++++++++++++++++++++++
It's good that this is being presented as library code, rather than
being buried in some device-specific directory then copied and pasted
ten times.
But ./lib/ does seem rather a strange place for it. Perhaps we need a
drivers/lib/ or something. We can use ./lib/ for now - it can always
be moved later on.
> --- /dev/null
> +++ b/lib/stmp_device.c
The functions in this file look terribly racy on SMP, or even with
preemption or interrupts. What happens if two CPUs or threads run
stmp_reset_block() against the same device at the same time?
Perhaps the caller is supposed to prevent that, and the documentation
which isn't there forgot to mention it ;)
next prev parent reply other threads:[~2012-04-23 6:53 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-21 22:21 [PATCH V2 0/3] introduce stmp-style devices Wolfram Sang
2012-03-21 22:21 ` Wolfram Sang
2012-03-21 22:21 ` [PATCH V2 1/3] lib: add support for " Wolfram Sang
2012-03-21 22:21 ` Wolfram Sang
2012-03-29 2:45 ` Huang Shijie
2012-03-29 2:45 ` Huang Shijie
2012-03-29 6:43 ` Wolfram Sang
2012-03-29 6:43 ` Wolfram Sang
2012-04-18 9:05 ` Dong Aisheng
2012-04-18 9:05 ` Dong Aisheng
2012-04-18 9:20 ` Wolfram Sang
2012-04-18 9:20 ` Wolfram Sang
2012-04-19 16:24 ` Arnd Bergmann
2012-04-19 16:24 ` Arnd Bergmann
2012-04-20 5:30 ` Shawn Guo
2012-04-20 5:30 ` Shawn Guo
2012-04-20 21:11 ` Wolfram Sang
2012-04-20 21:11 ` Wolfram Sang
2012-04-21 11:09 ` Arnd Bergmann
2012-04-21 11:09 ` Arnd Bergmann
2012-04-04 11:21 ` Huang Shijie
2012-04-04 11:21 ` Huang Shijie
2012-04-04 12:23 ` Wolfram Sang
2012-04-04 12:23 ` Wolfram Sang
2012-04-06 7:40 ` Huang Shijie
2012-04-06 7:40 ` Huang Shijie
2012-04-06 18:21 ` Wolfram Sang
2012-04-06 18:21 ` Wolfram Sang
2012-04-07 2:30 ` Huang Shijie
2012-04-07 2:30 ` Huang Shijie
2012-04-07 8:00 ` Wolfram Sang
2012-04-07 8:00 ` Wolfram Sang
2012-04-07 13:45 ` Huang Shijie
2012-04-07 13:45 ` Huang Shijie
2012-04-23 6:55 ` Andrew Morton [this message]
2012-04-23 6:55 ` Andrew Morton
2012-04-23 7:28 ` Wolfram Sang
2012-04-23 7:28 ` Wolfram Sang
2012-04-23 11:06 ` Arnd Bergmann
2012-04-23 11:06 ` Arnd Bergmann
2012-03-21 22:21 ` [PATCH 2/3] i2c: mxs: use global reset function Wolfram Sang
2012-03-21 22:21 ` Wolfram Sang
2012-03-21 22:21 ` [PATCH 3/3] rtc: stmp3xxx: use global stmp_device functionality Wolfram Sang
2012-03-21 22:21 ` Wolfram Sang
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=20120422235506.d3377f0e.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=b32955@freescale.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=shawn.guo@linaro.org \
--cc=w.sang@pengutronix.de \
/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.