* Applied "ASoC: sun4i-i2s: Add support for TDM slots" to the asoc tree
From: Mark Brown @ 2019-08-21 12:15 UTC (permalink / raw)
To: Maxime Ripard
Cc: alsa-devel, lgirdwood, Maxime Ripard, linux-kernel, codekipper,
Chen-Yu Tsai, Mark Brown, linux-arm-kernel
In-Reply-To: <26392af30b3e7b31ee48d5b867d45be8675db046.1566242458.git-series.maxime.ripard@bootlin.com>
The patch
ASoC: sun4i-i2s: Add support for TDM slots
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 137befe19f310400a8b20fd8a4ce8c4141aafde0 Mon Sep 17 00:00:00 2001
From: Maxime Ripard <maxime.ripard@bootlin.com>
Date: Mon, 19 Aug 2019 21:25:27 +0200
Subject: [PATCH] ASoC: sun4i-i2s: Add support for TDM slots
The i2s controller supports TDM, for up to 8 slots. Let's support the TDM
API.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Link: https://lore.kernel.org/r/26392af30b3e7b31ee48d5b867d45be8675db046.1566242458.git-series.maxime.ripard@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/sunxi/sun4i-i2s.c | 40 +++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 9e691baee1e8..8326b8cfa569 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -168,6 +168,8 @@ struct sun4i_i2s {
struct reset_control *rst;
unsigned int mclk_freq;
+ unsigned int slots;
+ unsigned int slot_width;
struct snd_dmaengine_dai_dma_data capture_dma_data;
struct snd_dmaengine_dai_dma_data playback_dma_data;
@@ -287,7 +289,7 @@ static bool sun4i_i2s_oversample_is_valid(unsigned int oversample)
static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
unsigned int rate,
- unsigned int channels,
+ unsigned int slots,
unsigned int word_size)
{
struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
@@ -335,7 +337,7 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
bclk_parent_rate = i2s->variant->get_bclk_parent_rate(i2s);
bclk_div = sun4i_i2s_get_bclk_div(i2s, bclk_parent_rate,
- rate, channels, word_size);
+ rate, slots, word_size);
if (bclk_div < 0) {
dev_err(dai->dev, "Unsupported BCLK divider: %d\n", bclk_div);
return -EINVAL;
@@ -419,6 +421,10 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
const struct snd_pcm_hw_params *params)
{
unsigned int channels = params_channels(params);
+ unsigned int slots = channels;
+
+ if (i2s->slots)
+ slots = i2s->slots;
/* Map the channels for playback and capture */
regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
@@ -428,7 +434,6 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
SUN4I_I2S_CHAN_SEL_MASK,
SUN4I_I2S_CHAN_SEL(channels));
-
regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
SUN4I_I2S_CHAN_SEL_MASK,
SUN4I_I2S_CHAN_SEL(channels));
@@ -452,10 +457,18 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+ unsigned int word_size = params_width(params);
unsigned int channels = params_channels(params);
+ unsigned int slots = channels;
int ret, sr, wss;
u32 width;
+ if (i2s->slots)
+ slots = i2s->slots;
+
+ if (i2s->slot_width)
+ word_size = i2s->slot_width;
+
ret = i2s->variant->set_chan_cfg(i2s, params);
if (ret < 0) {
dev_err(dai->dev, "Invalid channel configuration\n");
@@ -477,15 +490,14 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
if (sr < 0)
return -EINVAL;
- wss = i2s->variant->get_wss(i2s, params_width(params));
+ wss = i2s->variant->get_wss(i2s, word_size);
if (wss < 0)
return -EINVAL;
regmap_field_write(i2s->field_fmt_wss, wss);
regmap_field_write(i2s->field_fmt_sr, sr);
- return sun4i_i2s_set_clk_rate(dai, params_rate(params),
- channels, params_width(params));
+ return sun4i_i2s_set_clk_rate(dai, params_rate(params), slots, word_size);
}
static int sun4i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
@@ -785,10 +797,26 @@ static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
return 0;
}
+static int sun4i_i2s_set_tdm_slot(struct snd_soc_dai *dai,
+ unsigned int tx_mask, unsigned int rx_mask,
+ int slots, int slot_width)
+{
+ struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+
+ if (slots > 8)
+ return -EINVAL;
+
+ i2s->slots = slots;
+ i2s->slot_width = slot_width;
+
+ return 0;
+}
+
static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
.hw_params = sun4i_i2s_hw_params,
.set_fmt = sun4i_i2s_set_fmt,
.set_sysclk = sun4i_i2s_set_sysclk,
+ .set_tdm_slot = sun4i_i2s_set_tdm_slot,
.trigger = sun4i_i2s_trigger,
};
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 21/21] ASoC: sun4i-i2s: Add support for DSP formats
From: Mark Brown @ 2019-08-21 12:15 UTC (permalink / raw)
To: Maxime Ripard
Cc: alsa-devel, lgirdwood, linux-kernel, codekipper, Chen-Yu Tsai,
linux-arm-kernel
In-Reply-To: <74cc9562e056627e14f186089d349022b65f59e7.1566242458.git-series.maxime.ripard@bootlin.com>
[-- Attachment #1.1: Type: text/plain, Size: 282 bytes --]
On Mon, Aug 19, 2019 at 09:25:28PM +0200, Maxime Ripard wrote:
> From: Maxime Ripard <maxime.ripard@bootlin.com>
>
> In addition to the I2S format, the controller also supports the DSP_*
> formats.
This doesn't seem to apply against current code, please check and
resend.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH 0/2] nvmem: imx-ocotp: allow reads with arbitrary size and offset
From: Diaz de Grenu, Jose @ 2019-08-21 12:11 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: shawnguo@kernel.org, s.hauer@pengutronix.de,
linux-kernel@vger.kernel.org, linux-imx@nxp.com,
kernel@pengutronix.de, festevam@gmail.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <771a6f0a-3cc2-da20-2439-9a91dd2bf9d2@linaro.org>
On 06/08/2019 12:06 Srinivas Kandagatla wrote:
> Anyone form IMX can test this patchset before I push this out?
>
> Thanks,
> srini
Just for the record, I tested this on an i.MX6UL based board.
Let me know if there is something I can do to facilitate the testing to anyone from IMX.
Thanks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/4] iommu/io-pgtable-arm: Rationalise TCR handling
From: Will Deacon @ 2019-08-21 12:11 UTC (permalink / raw)
To: Robin Murphy; +Cc: robdclark, joro, jcrouse, iommu, linux-arm-kernel
In-Reply-To: <8cc47f43-ad74-b4e2-e977-6c78780abc91@arm.com>
On Tue, Aug 20, 2019 at 07:41:52PM +0100, Robin Murphy wrote:
> On 20/08/2019 17:07, Will Deacon wrote:
> > On Tue, Aug 20, 2019 at 04:25:56PM +0100, Robin Murphy wrote:
> > > On 20/08/2019 11:31, Will Deacon wrote:
> > > > On Mon, Aug 19, 2019 at 07:19:30PM +0100, Robin Murphy wrote:
> > > > > Although it's conceptually nice for the io_pgtable_cfg to provide a
> > > > > standard VMSA TCR value, the reality is that no VMSA-compliant IOMMU
> > > > > looks exactly like an Arm CPU, and they all have various other TCR
> > > > > controls which io-pgtable can't be expected to understand. Thus since
> > > > > there is an expectation that drivers will have to add to the given TCR
> > > > > value anyway, let's strip it down to just the essentials that are
> > > > > directly relevant to io-pgatble's inner workings - namely the address
> > > > > sizes, walk attributes, and where appropriate, format selection.
> > > > >
> > > > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > > > > ---
> > > > > drivers/iommu/arm-smmu-v3.c | 7 +------
> > > > > drivers/iommu/arm-smmu.c | 1 +
> > > > > drivers/iommu/arm-smmu.h | 2 ++
> > > > > drivers/iommu/io-pgtable-arm-v7s.c | 6 ++----
> > > > > drivers/iommu/io-pgtable-arm.c | 4 ----
> > > > > drivers/iommu/qcom_iommu.c | 2 +-
> > > > > 6 files changed, 7 insertions(+), 15 deletions(-)
> > > >
> > > > Hmm, so I'm a bit nervous about this one since I think we really should
> > > > be providing a TCR with EPD1 set if we're only giving you TTBR0. Relying
> > > > on the driver to do this worries me. See my comments on the next patch.
> > >
> > > The whole idea is that we already know we can't provide a *complete* TCR
> > > value (not least because anything above bit 31 is the wild west), thus
> > > there's really no point in io-pgtable trying to provide anything other than
> > > the parts it definitely controls. It makes sense to provide this partial TCR
> > > value "as if" for TTBR0, since that's the most common case, but ultimately
> > > io-pgatble doesn't know (or need to) which TTBR the caller intends to
> > > actually use for this table. Even if the caller *is* allocating it for
> > > TTBR0, io-pgtable doesn't know that they haven't got something live in TTBR1
> > > already, so it still wouldn't be in a position to make the EPD1 call either
> > > way.
> >
> > Ok, but the driver can happily rewrite/ignore what it gets back. I suppose
> > an alternative would be scrapped the 'u64 tcr' and instead having a bunch
> > of named bitfields for the stuff we're actually providing, although I'd
> > still like EPDx to be in there.
>
> I like the bitfield idea; it would certainly emphasise the "you have to do
> something more with this" angle that I'm pushing towards here, but still
> leave things framed in TCR terms without having to go to some more general
> abstraction. It really doesn't play into your EPD argument though - such a
> config would be providing TxSZ/TGx/IRGNx/ORGNx/SHx, but EPDy, for y = !x.
> For a driver to understand that and do the right thing with it is even more
> involved than for the driver to just set EPD1 by itself anyway.
Having considered the bitfield idea some more, I'm less attached to EPDx
because we simply wouldn't be making a statement about them, rather than a
(dangerous) zero value and expecting it to be ignored. So I think we're in
agreement on that.
The only part I'm still stuck to is that I think io-pgtable should know
whether it's targetting TTBR0 or TTBR1 so that it can sanitise input
addresses correctly. Doing this in the driver code is possible, but I'd
rather not start from that position, particularly as it would require things
like sign-extension in the TLBI callbacks.
> If only LPAE had created these bits as enables rather than disables then
> things would be logical and we could all be happy, but here we are...
I'm happy! :D:D:D
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/9] block: support diskcipher
From: Jens Axboe @ 2019-08-21 12:09 UTC (permalink / raw)
To: boojin.kim, linux-block, linux-kernel
Cc: 'Ulf Hansson', 'Mike Snitzer', dm-devel,
'Andreas Dilger', 'Alasdair Kergon',
'Eric Biggers', linux-samsung-soc, 'Herbert Xu',
'Krzysztof Kozlowski', 'Jaehoon Chung',
'Kukjin Kim', linux-ext4, 'Chao Yu',
linux-fscrypt, 'Jaegeuk Kim', linux-arm-kernel,
'Theodore Y. Ts'o', linux-mmc, linux-f2fs-devel,
linux-crypto, linux-fsdevel, 'David S. Miller'
In-Reply-To: <004101d557eb$98b00060$ca100120$@samsung.com>
On 8/21/19 12:42 AM, boojin.kim wrote:
> This patch supports crypto information to be maintained via BIO
> and passed to the storage driver.
>
> To do this, 'bi_aux_private', 'REQ_CYPTE' and 'bi_dun' are added
> to the block layer.
>
> 'bi_aux_private' is added for loading additional private information into
> BIO.
> 'REQ_CRYPT' is added to distinguish that bi_aux_private is being used
> for diskcipher.
> F2FS among encryption users uses DUN(device unit number) as
> the IV(initial vector) for cryptographic operations.
> DUN is stored in 'bi_dun' of bi_iter as a specific value for each BIO.
>
> Before attempting to merge the two BIOs, the operation is also added to
> verify that the crypto information contained in two BIOs is consistent.
This isn't going to happen. With this, and the inline encryption
proposed by Google, we'll bloat the bio even more. At least the Google
approach didn't include bio iter changes as well.
Please work it out between yourselves so we can have a single, clean
abstraction that works for both.
--
Jens Axboe
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 20/21] ASoC: sun4i-i2s: Add support for TDM slots
From: Mark Brown @ 2019-08-21 12:08 UTC (permalink / raw)
To: Maxime Ripard
Cc: alsa-devel, linux-kernel, lgirdwood, codekipper, Chen-Yu Tsai,
linux-arm-kernel, Sergey Suloev
In-Reply-To: <20190821120551.r4b3h4nnet357wem@flea>
[-- Attachment #1.1: Type: text/plain, Size: 972 bytes --]
On Wed, Aug 21, 2019 at 02:05:51PM +0200, Maxime Ripard wrote:
> On Tue, Aug 20, 2019 at 08:46:30AM +0300, Sergey Suloev wrote:
Please delete unneeded context from mails when replying. Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.
> > > .set_sysclk = sun4i_i2s_set_sysclk,
> > > + .set_tdm_slot = sun4i_i2s_set_tdm_slot,
> > > .trigger = sun4i_i2s_trigger,
> > > };
> > it seems like you forgot to implement sun4i_i2s_dai_ops.set_bclk_ratio
> > because, as I far as I understand, it should alter tdm slots functionality
> > indirectly.
> As far as I can see, while this indeed changes a few things on the TDM
> setup, it's optional, orthogonal and it has a single user in the tree
> (some intel platform card).
> So I'd say that if someone ever needs it, we can have it, but it's not
> a blocker.
Yes, that's a compltely orthogonal callback.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding
From: Maxime Ripard @ 2019-08-21 12:07 UTC (permalink / raw)
To: Samuel Holland
Cc: Mark Rutland, devicetree, linux-sunxi, Rob Herring, Stephen Boyd,
Michael Turquette, Jassi Brar, linux-kernel, Chen-Yu Tsai,
Rob Herring, Corentin Labbe, linux-clk, linux-arm-kernel
In-Reply-To: <8947f4d1-3bb4-11b8-b114-5016339514b8@sholland.org>
[-- Attachment #1.1: Type: text/plain, Size: 1012 bytes --]
On Tue, Aug 20, 2019 at 08:04:26AM -0500, Samuel Holland wrote:
> On 8/20/19 2:14 AM, Maxime Ripard wrote:
> > Hi,
> >
> > On Mon, Aug 19, 2019 at 10:23:04PM -0500, Samuel Holland wrote:
> >> This mailbox hardware is present in Allwinner sun8i, sun9i, and sun50i
> >> SoCs. Add a device tree binding for it.
> >>
> >> Reviewed-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >> ---
> >> .../mailbox/allwinner,sunxi-msgbox.yaml | 79 +++++++++++++++++++
> >> 1 file changed, 79 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
> >
> > So we merged a bunch of schemas already, with the convention that the
> > name was the first compatible to use that binding.
> >
> > That would be allwinner,sun6i-a31-msgbox.yaml in that case
>
> Okay, I'll rename the binding and driver (and Kconfig symbol?).
Yep, thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 20/21] ASoC: sun4i-i2s: Add support for TDM slots
From: Maxime Ripard @ 2019-08-21 12:05 UTC (permalink / raw)
To: Sergey Suloev
Cc: alsa-devel, lgirdwood, linux-kernel, codekipper, Chen-Yu Tsai,
broonie, linux-arm-kernel
In-Reply-To: <c311e88a-fdd2-8a01-275e-675d98dc90ba@orpaltech.com>
[-- Attachment #1.1: Type: text/plain, Size: 5143 bytes --]
Hi,
On Tue, Aug 20, 2019 at 08:46:30AM +0300, Sergey Suloev wrote:
> Hi, Maxime,
>
> On 8/19/19 10:25 PM, Maxime Ripard wrote:
> > From: Maxime Ripard <maxime.ripard@bootlin.com>
> >
> > The i2s controller supports TDM, for up to 8 slots. Let's support the TDM
> > API.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> > sound/soc/sunxi/sun4i-i2s.c | 40 ++++++++++++++++++++++++++++++++------
> > 1 file changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > index 0dac09814b65..4f76daeaaed7 100644
> > --- a/sound/soc/sunxi/sun4i-i2s.c
> > +++ b/sound/soc/sunxi/sun4i-i2s.c
> > @@ -168,6 +168,8 @@ struct sun4i_i2s {
> > struct reset_control *rst;
> > unsigned int mclk_freq;
> > + unsigned int slots;
> > + unsigned int slot_width;
> > struct snd_dmaengine_dai_dma_data capture_dma_data;
> > struct snd_dmaengine_dai_dma_data playback_dma_data;
> > @@ -287,7 +289,7 @@ static bool sun4i_i2s_oversample_is_valid(unsigned int oversample)
> > static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
> > unsigned int rate,
> > - unsigned int channels,
> > + unsigned int slots,
> > unsigned int word_size)
> > {
> > struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> > @@ -335,7 +337,7 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
> > bclk_parent_rate = i2s->variant->get_bclk_parent_rate(i2s);
> > bclk_div = sun4i_i2s_get_bclk_div(i2s, bclk_parent_rate,
> > - rate, channels, word_size);
> > + rate, slots, word_size);
> > if (bclk_div < 0) {
> > dev_err(dai->dev, "Unsupported BCLK divider: %d\n", bclk_div);
> > return -EINVAL;
> > @@ -419,6 +421,10 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> > const struct snd_pcm_hw_params *params)
> > {
> > unsigned int channels = params_channels(params);
> > + unsigned int slots = channels;
> > +
> > + if (i2s->slots)
> > + slots = i2s->slots;
> > /* Map the channels for playback and capture */
> > regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
> > @@ -428,7 +434,6 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> > regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> > SUN4I_I2S_CHAN_SEL_MASK,
> > SUN4I_I2S_CHAN_SEL(channels));
> > -
> > regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
> > SUN4I_I2S_CHAN_SEL_MASK,
> > SUN4I_I2S_CHAN_SEL(channels));
> > @@ -452,10 +457,18 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
> > struct snd_soc_dai *dai)
> > {
> > struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> > + unsigned int word_size = params_width(params);
> > unsigned int channels = params_channels(params);
> > + unsigned int slots = channels;
> > int ret, sr, wss;
> > u32 width;
> > + if (i2s->slots)
> > + slots = i2s->slots;
> > +
> > + if (i2s->slot_width)
> > + word_size = i2s->slot_width;
> > +
> > ret = i2s->variant->set_chan_cfg(i2s, params);
> > if (ret < 0) {
> > dev_err(dai->dev, "Invalid channel configuration\n");
> > @@ -477,15 +490,14 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
> > if (sr < 0)
> > return -EINVAL;
> > - wss = i2s->variant->get_wss(i2s, params_width(params));
> > + wss = i2s->variant->get_wss(i2s, word_size);
> > if (wss < 0)
> > return -EINVAL;
> > regmap_field_write(i2s->field_fmt_wss, wss);
> > regmap_field_write(i2s->field_fmt_sr, sr);
> > - return sun4i_i2s_set_clk_rate(dai, params_rate(params),
> > - channels, params_width(params));
> > + return sun4i_i2s_set_clk_rate(dai, params_rate(params), slots, word_size);
> > }
> > static int sun4i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s,
> > @@ -785,10 +797,26 @@ static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
> > return 0;
> > }
> > +static int sun4i_i2s_set_tdm_slot(struct snd_soc_dai *dai,
> > + unsigned int tx_mask, unsigned int rx_mask,
> > + int slots, int slot_width)
> > +{
> > + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> > +
> > + if (slots > 8)
> > + return -EINVAL;
> > +
> > + i2s->slots = slots;
> > + i2s->slot_width = slot_width;
> > +
> > + return 0;
> > +}
> > +
> > static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
> > .hw_params = sun4i_i2s_hw_params,
> > .set_fmt = sun4i_i2s_set_fmt,
> > .set_sysclk = sun4i_i2s_set_sysclk,
> > + .set_tdm_slot = sun4i_i2s_set_tdm_slot,
> > .trigger = sun4i_i2s_trigger,
> > };
>
> it seems like you forgot to implement sun4i_i2s_dai_ops.set_bclk_ratio
> because, as I far as I understand, it should alter tdm slots functionality
> indirectly.
As far as I can see, while this indeed changes a few things on the TDM
setup, it's optional, orthogonal and it has a single user in the tree
(some intel platform card).
So I'd say that if someone ever needs it, we can have it, but it's not
a blocker.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 16/59] KVM: arm64: nv: Save/Restore vEL2 sysregs
From: Alexandru Elisei @ 2019-08-21 11:57 UTC (permalink / raw)
To: Marc Zyngier, linux-arm-kernel, kvmarm, kvm; +Cc: Andre Przywara, Dave Martin
In-Reply-To: <20190621093843.220980-17-marc.zyngier@arm.com>
On 6/21/19 10:38 AM, Marc Zyngier wrote:
> From: Andre Przywara <andre.przywara@arm.com>
>
> Whenever we need to restore the guest's system registers to the CPU, we
> now need to take care of the EL2 system registers as well. Most of them
> are accessed via traps only, but some have an immediate effect and also
> a guest running in VHE mode would expect them to be accessible via their
> EL1 encoding, which we do not trap.
>
> Split the current __sysreg_{save,restore}_el1_state() functions into
> handling common sysregs, then differentiate between the guest running in
> vEL2 and vEL1.
>
> For vEL2 we write the virtual EL2 registers with an identical format directly
> into their EL1 counterpart, and translate the few registers that have a
> different format for the same effect on the execution when running a
> non-VHE guest guest hypervisor.
>
> [ Commit message reworked and many bug fixes applied by Marc Zyngier
> and Christoffer Dall. ]
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
> ---
> arch/arm64/kvm/hyp/sysreg-sr.c | 160 +++++++++++++++++++++++++++++++--
> 1 file changed, 153 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 62866a68e852..2abb9c3ff24f 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -22,6 +22,7 @@
> #include <asm/kvm_asm.h>
> #include <asm/kvm_emulate.h>
> #include <asm/kvm_hyp.h>
> +#include <asm/kvm_nested.h>
>
> /*
> * Non-VHE: Both host and guest must save everything.
> @@ -51,11 +52,9 @@ static void __hyp_text __sysreg_save_user_state(struct kvm_cpu_context *ctxt)
> ctxt->sys_regs[TPIDRRO_EL0] = read_sysreg(tpidrro_el0);
> }
>
> -static void __hyp_text __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
> +static void __hyp_text __sysreg_save_vel1_state(struct kvm_cpu_context *ctxt)
> {
> - ctxt->sys_regs[CSSELR_EL1] = read_sysreg(csselr_el1);
> ctxt->sys_regs[SCTLR_EL1] = read_sysreg_el1(SYS_SCTLR);
> - ctxt->sys_regs[ACTLR_EL1] = read_sysreg(actlr_el1);
> ctxt->sys_regs[CPACR_EL1] = read_sysreg_el1(SYS_CPACR);
> ctxt->sys_regs[TTBR0_EL1] = read_sysreg_el1(SYS_TTBR0);
> ctxt->sys_regs[TTBR1_EL1] = read_sysreg_el1(SYS_TTBR1);
> @@ -69,14 +68,58 @@ static void __hyp_text __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
> ctxt->sys_regs[CONTEXTIDR_EL1] = read_sysreg_el1(SYS_CONTEXTIDR);
> ctxt->sys_regs[AMAIR_EL1] = read_sysreg_el1(SYS_AMAIR);
> ctxt->sys_regs[CNTKCTL_EL1] = read_sysreg_el1(SYS_CNTKCTL);
> - ctxt->sys_regs[PAR_EL1] = read_sysreg(par_el1);
> - ctxt->sys_regs[TPIDR_EL1] = read_sysreg(tpidr_el1);
>
> ctxt->gp_regs.sp_el1 = read_sysreg(sp_el1);
> ctxt->gp_regs.elr_el1 = read_sysreg_el1(SYS_ELR);
> ctxt->gp_regs.spsr[KVM_SPSR_EL1]= read_sysreg_el1(SYS_SPSR);
> }
>
> +static void __sysreg_save_vel2_state(struct kvm_cpu_context *ctxt)
> +{
> + ctxt->sys_regs[ESR_EL2] = read_sysreg_el1(SYS_ESR);
> + ctxt->sys_regs[AFSR0_EL2] = read_sysreg_el1(SYS_AFSR0);
> + ctxt->sys_regs[AFSR1_EL2] = read_sysreg_el1(SYS_AFSR1);
> + ctxt->sys_regs[FAR_EL2] = read_sysreg_el1(SYS_FAR);
> + ctxt->sys_regs[MAIR_EL2] = read_sysreg_el1(SYS_MAIR);
> + ctxt->sys_regs[VBAR_EL2] = read_sysreg_el1(SYS_VBAR);
> + ctxt->sys_regs[CONTEXTIDR_EL2] = read_sysreg_el1(SYS_CONTEXTIDR);
> + ctxt->sys_regs[AMAIR_EL2] = read_sysreg_el1(SYS_AMAIR);
> +
> + /*
> + * In VHE mode those registers are compatible between EL1 and EL2,
> + * and the guest uses the _EL1 versions on the CPU naturally.
> + * So we save them into their _EL2 versions here.
> + * For nVHE mode we trap accesses to those registers, so our
> + * _EL2 copy in sys_regs[] is always up-to-date and we don't need
> + * to save anything here.
> + */
> + if (__vcpu_el2_e2h_is_set(ctxt)) {
> + ctxt->sys_regs[SCTLR_EL2] = read_sysreg_el1(SYS_SCTLR);
> + ctxt->sys_regs[CPTR_EL2] = read_sysreg_el1(SYS_CPACR);
> + ctxt->sys_regs[TTBR0_EL2] = read_sysreg_el1(SYS_TTBR0);
> + ctxt->sys_regs[TTBR1_EL2] = read_sysreg_el1(SYS_TTBR1);
> + ctxt->sys_regs[TCR_EL2] = read_sysreg_el1(SYS_TCR);
> + ctxt->sys_regs[CNTHCTL_EL2] = read_sysreg_el1(SYS_CNTKCTL);
> + }
This can break guests that run with VHE on, then disable it. I stumbled into
this while working on kvm-unit-tests, which uses TTBR0 for the translation
tables. Let's consider the following scenario:
1. Guest sets HCR_EL2.E2H
2. Guest programs translation tables in TTBR0_EL1, which should reflect in
TTBR0_EL2.
3. Guest enabled MMU and does stuff.
4. Guest disables MMU and clears HCR_EL2.E2H
5. Guest turns MMU on. It doesn't change TTBR0_EL2, because it will use the same
translation tables as when running with E2H set.
6. The vcpu gets scheduled out. E2H is not set, so the value that the guest
programmed in hardware TTBR0_EL1 won't be copied to virtual TTBR0_EL2.
7. The vcpu gets scheduled back in. KVM will write the reset value for virtual
TTBR0_EL2 (which is 0x0).
8. The guest hangs.
I think this is actually a symptom of a deeper issue. When E2H is cleared, the
values that the guest wrote to the EL1 registers aren't immediately reflected in
the virtual EL2 registers, as it happens on real hardware. Instead, some of the
hardware values from the EL1 registers are copied to the corresponding EL2
registers on the next vcpu_put, which happens at a later time.
I am thinking that something like this will fix the issues (it did fix disabling
VHE in kvm-unit-tests):
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1d896113f1f8..f2b5a39762d0 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -333,7 +333,8 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
* to reverse-translate virtual EL2 system registers for a
* non-VHE guest hypervisor.
*/
- __vcpu_sys_reg(vcpu, reg) = val;
+ if (reg != HCR_EL2)
+ __vcpu_sys_reg(vcpu, reg) = val;
switch (reg) {
case ELR_EL2:
@@ -370,7 +371,17 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int
reg)
return;
memory_write:
- __vcpu_sys_reg(vcpu, reg) = val;
+ if (reg == HCR_EL2 && vcpu_el2_e2h_is_set(vcpu) && !(val & HCR_E2H)) {
+ preempt_disable();
+ kvm_arch_vcpu_put(vcpu);
+
+ __vcpu_sys_reg(vcpu, reg) = val;
+
+ kvm_arch_vcpu_load(vcpu, smp_processor_id());
+ preempt_enable();
+ } else {
+ __vcpu_sys_reg(vcpu, reg) = val;
+ }
}
/* 3 bits per cache level, as per CLIDR, but non-existent caches always 0 */
I don't think there's any need to convert EL1 registers to their non-vhe EL2
format because of how RES0/RES1 is defined in the architecture glossary (ARM DDI
0487E.a, page 7893 for RES0 and 7894 for RES1):
"If a bit is RES0 only in some contexts:
A read of the bit must return the value last successfully written to the bit, by
either a direct or an indirect write, regardless of the use of the register when
the bit was written
[..]
While the use of the register is such that the bit is described as RES0, the
value of the bit must have no effect on the operation of the PE, other than
determining the value read back from that bit, unless this Manual explicitly
defines additional properties for the bit"
We have the translate functions that should take care of converting the non-vhe
EL2 format to the hardware EL1 format.
As an aside, the diff looks weird because the vcpu_write_sys_reg is very
complex, there are a LOT of exit points from the function, and the register
value is written twice for some registers. I think it's worth considering making
the function simpler, maybe splitting it into two separate functions, one for
EL2 registers, one for regular registers.
Here's the kvm-unit-tests diff that I used to spot the bug. It's very far from
being correct, but the test is able to finish with the fix (it hangs otherwise).
You can apply it on top of 2130fd4154ad ("tscdeadline_latency: Check condition
first before loop"):
diff --git a/arm/cstart64.S b/arm/cstart64.S
index b0e8baa1a23a..01357b3b116b 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -51,6 +51,18 @@ start:
b 1b
1:
+ mrs x4, CurrentEL
+ cmp x4, CurrentEL_EL2
+ b.ne 1f
+ mrs x4, mpidr_el1
+ msr vmpidr_el2, x4
+ mrs x4, midr_el1
+ msr vpidr_el2, x4
+ ldr x4, =(HCR_EL2_TGE | HCR_EL2_E2H)
+ msr hcr_el2, x4
+ isb
+
+1:
/* set up stack */
mov x4, #1
msr spsel, x4
@@ -101,6 +113,18 @@ get_mmu_off:
.globl secondary_entry
secondary_entry:
+ mrs x0, CurrentEL
+ cmp x0, CurrentEL_EL2
+ b.ne 1f
+ mrs x0, mpidr_el1
+ msr vmpidr_el2, x0
+ mrs x0, midr_el1
+ msr vpidr_el2, x0
+ ldr x0, =(HCR_EL2_TGE | HCR_EL2_E2H)
+ msr hcr_el2, x0
+ isb
+
+1:
/* Enable FP/ASIMD */
mov x0, #(3 << 20)
msr cpacr_el1, x0
@@ -194,6 +218,33 @@ asm_mmu_enable:
ret
+asm_mmu_enable_hyp:
+ ic iallu
+ tlbi alle2is
+ dsb ish
+
+ /* TCR */
+ ldr x1, =TCR_EL2_RES1 | \
+ TCR_T0SZ(VA_BITS) | \
+ TCR_TG0_64K | \
+ TCR_IRGN0_WBWA | TCR_ORGN0_WBWA | \
+ TCR_SH0_IS
+ mrs x2, id_aa64mmfr0_el1
+ bfi x1, x2, #TCR_EL2_PS_SHIFT, #3
+ msr tcr_el2, x1
+
+ /* Same MAIR and TTBR0 as in VHE mode */
+
+ /* SCTLR */
+ ldr x1, =SCTLR_EL2_RES1 | \
+ SCTLR_EL2_C | \
+ SCTLR_EL2_I | \
+ SCTLR_EL2_M
+ msr sctlr_el2, x1
+ isb
+
+ ret
+
.globl asm_mmu_disable
asm_mmu_disable:
mrs x0, sctlr_el1
@@ -202,6 +253,18 @@ asm_mmu_disable:
isb
ret
+.globl asm_disable_vhe
+asm_disable_vhe:
+ str x30, [sp, #-16]!
+
+ bl asm_mmu_disable
+ msr hcr_el2, xzr
+ isb
+ bl asm_mmu_enable_hyp
+
+ ldr x30, [sp], #16
+ ret
+
/*
* Vectors
* Adapted from arch/arm64/kernel/entry.S
diff --git a/arm/selftest.c b/arm/selftest.c
index 28a17f7a7531..68a18036221b 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -287,6 +287,12 @@ static void user_psci_system_off(struct pt_regs *regs,
unsigned int esr)
{
__user_psci_system_off();
}
+
+extern void asm_disable_vhe(void);
+static void check_el2(void)
+{
+ asm_disable_vhe();
+}
#endif
static void check_vectors(void *arg __unused)
@@ -369,6 +375,10 @@ int main(int argc, char **argv)
report("PSCI version", psci_check());
on_cpus(cpu_report, NULL);
+ } else if (strcmp(argv[1], "el2") == 0) {
+
+ check_el2();
+
} else {
printf("Unknown subtest\n");
abort();
diff --git a/lib/arm/psci.c b/lib/arm/psci.c
index c3d399064ae3..5ef1c0386ce1 100644
--- a/lib/arm/psci.c
+++ b/lib/arm/psci.c
@@ -16,7 +16,7 @@ int psci_invoke(unsigned long function_id, unsigned long arg0,
unsigned long arg1, unsigned long arg2)
{
asm volatile(
- "hvc #0"
+ "smc #0"
: "+r" (function_id)
: "r" (arg0), "r" (arg1), "r" (arg2));
return function_id;
diff --git a/lib/arm64/asm/pgtable-hwdef.h b/lib/arm64/asm/pgtable-hwdef.h
index 045a3ce12645..6b80e34dda0c 100644
--- a/lib/arm64/asm/pgtable-hwdef.h
+++ b/lib/arm64/asm/pgtable-hwdef.h
@@ -95,18 +95,42 @@
/*
* TCR flags.
*/
-#define TCR_TxSZ(x) (((UL(64) - (x)) << 16) | ((UL(64) - (x)) << 0))
-#define TCR_IRGN_NC ((UL(0) << 8) | (UL(0) << 24))
-#define TCR_IRGN_WBWA ((UL(1) << 8) | (UL(1) << 24))
-#define TCR_IRGN_WT ((UL(2) << 8) | (UL(2) << 24))
-#define TCR_IRGN_WBnWA ((UL(3) << 8) | (UL(3) << 24))
-#define TCR_IRGN_MASK ((UL(3) << 8) | (UL(3) << 24))
-#define TCR_ORGN_NC ((UL(0) << 10) | (UL(0) << 26))
-#define TCR_ORGN_WBWA ((UL(1) << 10) | (UL(1) << 26))
-#define TCR_ORGN_WT ((UL(2) << 10) | (UL(2) << 26))
-#define TCR_ORGN_WBnWA ((UL(3) << 10) | (UL(3) << 26))
-#define TCR_ORGN_MASK ((UL(3) << 10) | (UL(3) << 26))
-#define TCR_SHARED ((UL(3) << 12) | (UL(3) << 28))
+#define TCR_T0SZ(x) ((UL(64) - (x)) << 0)
+#define TCR_T1SZ(x) ((UL(64) - (x)) << 16)
+#define TCR_TxSZ(x) (TCR_T0SZ(x) | TCR_T1SZ(x))
+#define TCR_IRGN0_NC (UL(0) << 8)
+#define TCR_IRGN1_NC (UL(0) << 24)
+#define TCR_IRGN_NC (TCR_IRGN0_NC | TCR_IRGN1_NC)
+#define TCR_IRGN0_WBWA (UL(1) << 8)
+#define TCR_IRGN1_WBWA (UL(1) << 24)
+#define TCR_IRGN_WBWA (TCR_IRGN0_WBWA | TCR_IRGN1_WBWA)
+#define TCR_IRGN0_WT (UL(2) << 8)
+#define TCR_IRGN1_WT (UL(2) << 24)
+#define TCR_IRGN_WT (TCR_IRGN0_WT | TCR_IRGN1_WT)
+#define TCR_IRGN0_WBnWA (UL(3) << 8)
+#define TCR_IRGN1_WBnWA (UL(3) << 24)
+#define TCR_IRGN_WBnWA (TCR_IRGN0_WBnWA | TCR_IRGN1_WBnWA)
+#define TCR_IRGN0_MASK (UL(3) << 8)
+#define TCR_IRGN1_MASK (UL(3) << 24)
+#define TCR_IRGN_MASK (TCR_IRGN0_MASK | TCR_IRGN1_MASK)
+#define TCR_ORGN0_NC (UL(0) << 10)
+#define TCR_ORGN1_NC (UL(0) << 26)
+#define TCR_ORGN_NC (TCR_ORGN0_NC | TCR_ORGN1_NC)
+#define TCR_ORGN0_WBWA (UL(1) << 10)
+#define TCR_ORGN1_WBWA (UL(1) << 26)
+#define TCR_ORGN_WBWA (TCR_ORGN0_WBWA | TCR_ORGN1_WBWA)
+#define TCR_ORGN0_WT (UL(2) << 10)
+#define TCR_ORGN1_WT (UL(2) << 26)
+#define TCR_ORGN_WT (TCR_ORGN0_WT | TCR_ORGN1_WT)
+#define TCR_ORGN0_WBnWA (UL(3) << 8)
+#define TCR_ORGN1_WBnWA (UL(3) << 24)
+#define TCR_ORGN_WBnWA (TCR_ORGN0_WBnWA | TCR_ORGN1_WBnWA)
+#define TCR_ORGN0_MASK (UL(3) << 10)
+#define TCR_ORGN1_MASK (UL(3) << 26)
+#define TCR_ORGN_MASK (TCR_ORGN0_MASK | TCR_ORGN1_MASK)
+#define TCR_SH0_IS (UL(3) << 12)
+#define TCR_SH1_IS (UL(3) << 28)
+#define TCR_SHARED (TCR_SH0_IS | TCR_SH1_IS)
#define TCR_TG0_4K (UL(0) << 14)
#define TCR_TG0_64K (UL(1) << 14)
#define TCR_TG0_16K (UL(2) << 14)
@@ -116,6 +140,9 @@
#define TCR_ASID16 (UL(1) << 36)
#define TCR_TBI0 (UL(1) << 37)
+#define TCR_EL2_RES1 ((UL(1) << 31) | (UL(1) << 23))
+#define TCR_EL2_PS_SHIFT 16
+
/*
* Memory types available.
*/
diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index 1d9223f728a5..b2136acda743 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -16,6 +16,16 @@
#define SCTLR_EL1_A (1 << 1)
#define SCTLR_EL1_M (1 << 0)
+#define HCR_EL2_TGE (1 << 27)
+#define HCR_EL2_E2H (1 << 34)
+
+#define SCTLR_EL2_RES1 ((UL(3) << 28) | (UL(3) << 22) | \
+ (UL(1) << 18) | (UL(1) << 16) | \
+ (UL(1) << 11) | (UL(3) << 4))
+#define SCTLR_EL2_I SCTLR_EL1_I
+#define SCTLR_EL2_C SCTLR_EL1_C
+#define SCTLR_EL2_M SCTLR_EL1_M
+
#ifndef __ASSEMBLY__
#include <asm/ptrace.h>
#include <asm/esr.h>
To run it:
lkvm run -f selftest.flat -c 1 -m 128 -p el2 --nested --irqchip gicv3 --console
serial
Thanks,
Alex
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v7 1/5] dt-bindings: media: Add Allwinner A10 CSI binding
From: Maxime Ripard @ 2019-08-21 11:57 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mark Rutland, devicetree, Rob Herring, linux-kernel, Chen-Yu Tsai,
Rob Herring, Hans Verkuil, Laurent Pinchart, Thomas Petazzoni,
Mauro Carvalho Chehab, Frank Rowand, linux-arm-kernel,
linux-media
In-Reply-To: <20190820114849.GD5123@paasikivi.fi.intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 4165 bytes --]
Hi Sakari,
On Tue, Aug 20, 2019 at 02:48:49PM +0300, Sakari Ailus wrote:
> Hi Maxime,
>
> On Tue, Aug 20, 2019 at 01:24:32PM +0200, Maxime Ripard wrote:
> > From: Maxime Ripard <maxime.ripard@bootlin.com>
> >
> > The Allwinner A10 CMOS Sensor Interface is a camera capture interface also
> > used in later (A10s, A13, A20, R8 and GR8) SoCs.
> >
> > On some SoCs, like the A10, there's multiple instances of that controller,
> > with one instance supporting more channels and having an ISP.
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> > Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 107 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > new file mode 100644
> > index 000000000000..9000bca344f9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > @@ -0,0 +1,107 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings
> > +
> > +maintainers:
> > + - Chen-Yu Tsai <wens@csie.org>
> > + - Maxime Ripard <maxime.ripard@bootlin.com>
> > +
> > +description: |-
> > + The Allwinner A10 and later has a CMOS Sensor Interface to retrieve
> > + frames from a parallel or BT656 sensor.
> > +
> > +properties:
> > + compatible:
> > + const: allwinner,sun7i-a20-csi0
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: The CSI interface clock
> > + - description: The CSI module clock
> > + - description: The CSI ISP clock
> > + - description: The CSI DRAM clock
> > +
> > + clock-names:
> > + items:
> > + - const: bus
> > + - const: mod
> > + - const: isp
> > + - const: ram
> > +
> > + resets:
> > + maxItems: 1
> > +
> > + port:
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + endpoint:
> > + properties:
> > + bus-width:
> > + const: 8
> > + description: Number of data lines actively used.
>
> Are other values supported? If not, you could omit this.
It can also support 16 bits data input, but this description is
redundant anyway, I'll remove it.
> > +
> > + data-active: true
> > + hsync-active: true
> > + pclk-sample: true
> > + remote-endpoint: true
> > + vsync-active: true
> > +
> > + required:
> > + - bus-width
> > + - data-active
> > + - hsync-active
> > + - pclk-sample
> > + - remote-endpoint
> > + - vsync-active
>
> Some of these are not allowed in the Bt.656 mode (vsync-active and
> hsync-active) while they're required in Bt.601 mode. Is there a way to tell
> that in YAML-based bindings?
You could, but that would be more suited in another schemas. The way
schemas works is that you can have several layers of them, and each
being validated in isolation from the others.
Here, we're just listing the values usable by that binding, and it
will be used only to validate that binding.
Eventually, we'll want to have a video-interfaces schemas that will
apply to all the OF graph users, with those constraints defined.
This way, we can avoid a lot of duplication and just have the binding
description.
I guess I could just have the remote endpoint being required, and the
rest will be in the generic part.
> Similarly, video-interfaces.txt should be referenced from here, shouldn't
> it?
Sure.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/9] soc: samsung: Convert exynos-chipid driver to use the regmap API
From: Bartlomiej Zolnierkiewicz @ 2019-08-21 11:51 UTC (permalink / raw)
To: Sylwester Nawrocki, Krzysztof Kozlowski
Cc: devicetree, linux-samsung-soc@vger.kernel.org, Arnd Bergmann,
linux-pm, vireshk, linux-kernel@vger.kernel.org, Jon Hunter,
robh+dt, kgene, Sylwester Nawrocki, pankaj.dubey, linux-tegra,
linux-arm-kernel, Marek Szyprowski
In-Reply-To: <1e428c8e-f4b5-0810-77f9-2c899c040fc7@kernel.org>
Hi,
On 8/20/19 11:38 PM, Sylwester Nawrocki wrote:
> On 8/20/19 21:37, Krzysztof Kozlowski wrote:
>>>> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
>
>>>> @@ -51,29 +48,24 @@ static const char * __init product_id_to_soc_id(unsigned int product_id)
>>>> int __init exynos_chipid_early_init(void)
>>>> {
>>>> struct soc_device_attribute *soc_dev_attr;
>>>> - void __iomem *exynos_chipid_base;
>>>> struct soc_device *soc_dev;
>>>> struct device_node *root;
>>>> - struct device_node *np;
>>>> + struct regmap *regmap;
>>>> u32 product_id;
>>>> u32 revision;
>>>> + int ret;
>>>>
>>>> - /* look up for chipid node */
>>>> - np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
>>>> - if (!np)
>>>> - return -ENODEV;
>>>> -
>>>> - exynos_chipid_base = of_iomap(np, 0);
>>>> - of_node_put(np);
>>>> -
>>>> - if (!exynos_chipid_base) {
>>>> - pr_err("Failed to map SoC chipid\n");
>>>> - return -ENXIO;
>>>> + regmap = syscon_regmap_lookup_by_compatible("samsung,exynos4210-chipid");
>>>> + if (IS_ERR(regmap)) {
>>>> + pr_err("Failed to get CHIPID regmap\n");
>>>> + return PTR_ERR(regmap);
>>>> }
>>> Following this change, I am now seeing the above error on our Tegra
>>> boards where this driver is enabled. This is triggering a kernel
>>> warnings test we have to fail. Hence, I don't think that you can remove
>>> the compatible node test here, unless you have a better way to determine
>>> if this is a samsung device.
>>
>> Right, this is really wrong... I missed that it is not a probe but
>> early init. And this init will be called on every board... Probably it
>> should be converted to a regular driver.
Early initialization is needed for SoC driver to be used from within
arch/arm/mach-exynos/ and _initcall() usage is the usual way for SoC
drivers to be initialized:
drivers/soc/amlogic/meson-gx-socinfo.c
drivers/soc/amlogic/meson-mx-socinfo.c
drivers/soc/atmel/soc.c
drivers/soc/bcm/brcmstb/common.c
drivers/soc/imx/soc-imx-scu.c
drivers/soc/imx/soc-imx8.c
drivers/soc/renesas/renesas-soc.c
drivers/soc/tegra/fuse/fuse-tegra.c
drivers/soc/ux500/ux500-soc-id.c
drivers/soc/versatile/soc-integrator.c
drivers/soc/versatile/soc-integrator.c
The only SoC drivers that are regular drivers are:
drivers/soc/fsl/guts.c
drivers/soc/versatile/soc-realview.c
> I'm also inclined to have it converted to a regular driver. We already
> have "exynos-asv" driver matching on the chipid node (patch 3/9).
> The ASV patches will not be merged soon anyway, all this needs some more
> thought. Krzysztof, can we abandon the chipid patches for now? Your
chipid driver is good and useful on its own. The preferred solution
IMHO would be to just revert "soc: samsung: Convert exynos-chipid
driver to use the regmap API" commit.
> pull request doesn't appear to be merged to arm-soc yet. Sorry about
> that.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 5/5] arm64: dts: meson-sm1-sei610: add USB support
From: Neil Armstrong @ 2019-08-21 11:41 UTC (permalink / raw)
To: khilman, ulf.hansson
Cc: linux-amlogic, linux-pm, linux-kernel, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190821114121.10430-1-narmstrong@baylibre.com>
Add the USB properties for the Amlogic SM1 Based SEI610 Board in order to
support the USB DRD Type-C port and the USB3 Type A port.
The USB DRD Type-C controller uses the ID signal to toggle the USB role
between the DWC3 Host controller and the DWC2 Device controller.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
index 66bd3bfbaf91..36ac2e4b970d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
@@ -321,3 +321,8 @@
pinctrl-0 = <&uart_ao_a_pins>;
pinctrl-names = "default";
};
+
+&usb {
+ status = "okay";
+ dr_mode = "otg";
+};
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/5] dt-bindings: power: add Amlogic Everything-Else power domains bindings
From: Neil Armstrong @ 2019-08-21 11:41 UTC (permalink / raw)
To: khilman, ulf.hansson, devicetree
Cc: linux-amlogic, linux-pm, linux-kernel, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190821114121.10430-1-narmstrong@baylibre.com>
Add the bindings for the Amlogic Everything-Else power domains,
controlling the Everything-Else peripherals power domains.
The bindings targets the Amlogic G12A and SM1 compatible SoCs,
support for earlier SoCs will be added later.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../bindings/power/amlogic,meson-ee-pwrc.yaml | 93 +++++++++++++++++++
include/dt-bindings/power/meson-g12a-power.h | 13 +++
include/dt-bindings/power/meson-sm1-power.h | 18 ++++
3 files changed, 124 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-ee-pwrc.yaml
create mode 100644 include/dt-bindings/power/meson-g12a-power.h
create mode 100644 include/dt-bindings/power/meson-sm1-power.h
diff --git a/Documentation/devicetree/bindings/power/amlogic,meson-ee-pwrc.yaml b/Documentation/devicetree/bindings/power/amlogic,meson-ee-pwrc.yaml
new file mode 100644
index 000000000000..aab70e8b681e
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/amlogic,meson-ee-pwrc.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 BayLibre, SAS
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/power/amlogic,meson-ee-pwrc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic Meson Everything-Else Power Domains
+
+maintainers:
+ - Neil Armstrong <narmstrong@baylibre.com>
+
+description: |+
+ The Everything-Else Power Domains node should be the child of a syscon
+ node with the required property:
+
+ - compatible: Should be the following:
+ "amlogic,meson-gx-hhi-sysctrl", "simple-mfd", "syscon"
+
+ Refer to the the bindings described in
+ Documentation/devicetree/bindings/mfd/syscon.txt
+
+properties:
+ compatible:
+ enum:
+ - amlogic,meson-g12a-pwrc
+ - amlogic,meson-sm1-pwrc
+
+ clocks:
+ minItems: 2
+
+ clock-names:
+ items:
+ - const: vpu
+ - const: vapb
+
+ resets:
+ minItems: 11
+
+ reset-names:
+ items:
+ - const: viu
+ - const: venc
+ - const: vcbus
+ - const: bt656
+ - const: rdma
+ - const: venci
+ - const: vencp
+ - const: vdac
+ - const: vdi6
+ - const: vencl
+ - const: vid_lock
+
+ "#power-domain-cells":
+ const: 1
+
+ amlogic,ao-sysctrl:
+ description: phandle to the AO sysctrl node
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+ - compatible
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - "#power-domain-cells"
+ - amlogic,ao-sysctrl
+
+examples:
+ - |
+ pwrc: power-controller {
+ compatible = "amlogic,meson-sm1-pwrc";
+ #power-domain-cells = <1>;
+ amlogic,ao-sysctrl = <&rti>;
+ resets = <&reset_viu>,
+ <&reset_venc>,
+ <&reset_vcbus>,
+ <&reset_bt656>,
+ <&reset_rdma>,
+ <&reset_venci>,
+ <&reset_vencp>,
+ <&reset_vdac>,
+ <&reset_vdi6>,
+ <&reset_vencl>,
+ <&reset_vid_lock>;
+ reset-names = "viu", "venc", "vcbus", "bt656",
+ "rdma", "venci", "vencp", "vdac",
+ "vdi6", "vencl", "vid_lock";
+ clocks = <&clk_vpu>, <&clk_vapb>;
+ clock-names = "vpu", "vapb";
+ };
diff --git a/include/dt-bindings/power/meson-g12a-power.h b/include/dt-bindings/power/meson-g12a-power.h
new file mode 100644
index 000000000000..bb5e67a842de
--- /dev/null
+++ b/include/dt-bindings/power/meson-g12a-power.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */
+/*
+ * Copyright (c) 2019 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#ifndef _DT_BINDINGS_MESON_G12A_POWER_H
+#define _DT_BINDINGS_MESON_G12A_POWER_H
+
+#define PWRC_G12A_VPU_ID 0
+#define PWRC_G12A_ETH_ID 1
+
+#endif
diff --git a/include/dt-bindings/power/meson-sm1-power.h b/include/dt-bindings/power/meson-sm1-power.h
new file mode 100644
index 000000000000..a020ab00c134
--- /dev/null
+++ b/include/dt-bindings/power/meson-sm1-power.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */
+/*
+ * Copyright (c) 2019 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#ifndef _DT_BINDINGS_MESON_SM1_POWER_H
+#define _DT_BINDINGS_MESON_SM1_POWER_H
+
+#define PWRC_SM1_VPU_ID 0
+#define PWRC_SM1_NNA_ID 1
+#define PWRC_SM1_USB_ID 2
+#define PWRC_SM1_PCIE_ID 3
+#define PWRC_SM1_GE2D_ID 4
+#define PWRC_SM1_AUDIO_ID 5
+#define PWRC_SM1_ETH_ID 6
+
+#endif
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 2/5] soc: amlogic: Add support for Everything-Else power domains controller
From: Neil Armstrong @ 2019-08-21 11:41 UTC (permalink / raw)
To: khilman, ulf.hansson
Cc: linux-amlogic, linux-pm, linux-kernel, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190821114121.10430-1-narmstrong@baylibre.com>
Add support for the General Purpose Amlogic Everything-Else Power controller,
with the first support for G12A and SM1 SoCs dedicated to the VPU, PCIe,
USB, NNA, GE2D and Ethernet Power Domains.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/soc/amlogic/Kconfig | 11 +
drivers/soc/amlogic/Makefile | 1 +
drivers/soc/amlogic/meson-ee-pwrc.c | 560 ++++++++++++++++++++++++++++
3 files changed, 572 insertions(+)
create mode 100644 drivers/soc/amlogic/meson-ee-pwrc.c
diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig
index 23bfb8ef4fdb..bc2c912949bd 100644
--- a/drivers/soc/amlogic/Kconfig
+++ b/drivers/soc/amlogic/Kconfig
@@ -37,6 +37,17 @@ config MESON_GX_PM_DOMAINS
Say yes to expose Amlogic Meson GX Power Domains as
Generic Power Domains.
+config MESON_EE_PM_DOMAINS
+ bool "Amlogic Meson Everything-Else Power Domains driver"
+ depends on ARCH_MESON || COMPILE_TEST
+ depends on PM && OF
+ default ARCH_MESON
+ select PM_GENERIC_DOMAINS
+ select PM_GENERIC_DOMAINS_OF
+ help
+ Say yes to expose Amlogic Meson Everything-Else Power Domains as
+ Generic Power Domains.
+
config MESON_MX_SOCINFO
bool "Amlogic Meson MX SoC Information driver"
depends on ARCH_MESON || COMPILE_TEST
diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile
index f2e4ed171297..de79d044b545 100644
--- a/drivers/soc/amlogic/Makefile
+++ b/drivers/soc/amlogic/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_MESON_CLK_MEASURE) += meson-clk-measure.o
obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o
obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o
obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o
+obj-$(CONFIG_MESON_EE_PM_DOMAINS) += meson-ee-pwrc.o
diff --git a/drivers/soc/amlogic/meson-ee-pwrc.c b/drivers/soc/amlogic/meson-ee-pwrc.c
new file mode 100644
index 000000000000..7159888c850b
--- /dev/null
+++ b/drivers/soc/amlogic/meson-ee-pwrc.c
@@ -0,0 +1,560 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2019 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/bitfield.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/reset-controller.h>
+#include <linux/reset.h>
+#include <linux/clk.h>
+#include <dt-bindings/power/meson-g12a-power.h>
+#include <dt-bindings/power/meson-sm1-power.h>
+
+/* AO Offsets */
+
+#define AO_RTI_GEN_PWR_SLEEP0 (0x3a << 2)
+#define AO_RTI_GEN_PWR_ISO0 (0x3b << 2)
+
+/* HHI Offsets */
+
+#define HHI_MEM_PD_REG0 (0x40 << 2)
+#define HHI_VPU_MEM_PD_REG0 (0x41 << 2)
+#define HHI_VPU_MEM_PD_REG1 (0x42 << 2)
+#define HHI_VPU_MEM_PD_REG3 (0x43 << 2)
+#define HHI_VPU_MEM_PD_REG4 (0x44 << 2)
+#define HHI_AUDIO_MEM_PD_REG0 (0x45 << 2)
+#define HHI_NANOQ_MEM_PD_REG0 (0x46 << 2)
+#define HHI_NANOQ_MEM_PD_REG1 (0x47 << 2)
+#define HHI_VPU_MEM_PD_REG2 (0x4d << 2)
+
+struct meson_ee_pwrc;
+struct meson_ee_pwrc_domain;
+
+struct meson_ee_pwrc_mem_domain {
+ unsigned int reg;
+ unsigned int mask;
+};
+
+struct meson_ee_pwrc_top_domain {
+ unsigned int sleep_reg;
+ unsigned int sleep_mask;
+ unsigned int iso_reg;
+ unsigned int iso_mask;
+};
+
+struct meson_ee_pwrc_domain_desc {
+ char *name;
+ char **reset_names;
+ unsigned int reset_names_count;
+ char **clk_names;
+ unsigned int clk_names_count;
+ struct meson_ee_pwrc_top_domain *top_pd;
+ unsigned int mem_pd_count;
+ struct meson_ee_pwrc_mem_domain *mem_pd;
+ bool (*get_power)(struct meson_ee_pwrc_domain *pwrc_domain);
+};
+
+struct meson_ee_pwrc_domain_data {
+ unsigned int count;
+ struct meson_ee_pwrc_domain_desc *domains;
+};
+
+/* Clock and Resets lists */
+
+static char *g12a_pwrc_vpu_resets[] = {
+ "viu", "venc", "vcbus", "bt656",
+ "rdma", "venci", "vencp", "vdac",
+ "vdi6", "vencl", "vid_lock",
+};
+
+static char *g12a_pwrc_vpu_clks[] = {
+ "vpu", "vapb",
+};
+
+/* TOP Power Domains */
+
+static struct meson_ee_pwrc_top_domain g12a_pwrc_vpu = {
+ .sleep_reg = AO_RTI_GEN_PWR_SLEEP0,
+ .sleep_mask = BIT(8),
+ .iso_reg = AO_RTI_GEN_PWR_SLEEP0,
+ .iso_mask = BIT(9),
+};
+
+#define SM1_EE_PD(__bit) \
+ { \
+ .sleep_reg = AO_RTI_GEN_PWR_SLEEP0, \
+ .sleep_mask = BIT(__bit), \
+ .iso_reg = AO_RTI_GEN_PWR_ISO0, \
+ .iso_mask = BIT(__bit), \
+ }
+
+static struct meson_ee_pwrc_top_domain sm1_pwrc_vpu = SM1_EE_PD(8);
+static struct meson_ee_pwrc_top_domain sm1_pwrc_nna = SM1_EE_PD(16);
+static struct meson_ee_pwrc_top_domain sm1_pwrc_usb = SM1_EE_PD(17);
+static struct meson_ee_pwrc_top_domain sm1_pwrc_pci = SM1_EE_PD(18);
+static struct meson_ee_pwrc_top_domain sm1_pwrc_ge2d = SM1_EE_PD(19);
+
+/* Memory PD Domains */
+
+#define VPU_MEMPD(__reg) \
+ { __reg, GENMASK(1, 0) }, \
+ { __reg, GENMASK(3, 2) }, \
+ { __reg, GENMASK(5, 4) }, \
+ { __reg, GENMASK(7, 6) }, \
+ { __reg, GENMASK(9, 8) }, \
+ { __reg, GENMASK(11, 10) }, \
+ { __reg, GENMASK(13, 12) }, \
+ { __reg, GENMASK(15, 14) }, \
+ { __reg, GENMASK(17, 16) }, \
+ { __reg, GENMASK(19, 18) }, \
+ { __reg, GENMASK(21, 20) }, \
+ { __reg, GENMASK(23, 22) }, \
+ { __reg, GENMASK(25, 24) }, \
+ { __reg, GENMASK(27, 26) }, \
+ { __reg, GENMASK(29, 28) }, \
+ { __reg, GENMASK(31, 30) }
+
+#define VPU_HHI_MEMPD(__reg) \
+ { __reg, BIT(8) }, \
+ { __reg, BIT(9) }, \
+ { __reg, BIT(10) }, \
+ { __reg, BIT(11) }, \
+ { __reg, BIT(12) }, \
+ { __reg, BIT(13) }, \
+ { __reg, BIT(14) }, \
+ { __reg, BIT(15) }
+
+static struct meson_ee_pwrc_mem_domain g12a_pwrc_mem_vpu[] = {
+ VPU_MEMPD(HHI_VPU_MEM_PD_REG0),
+ VPU_MEMPD(HHI_VPU_MEM_PD_REG1),
+ VPU_MEMPD(HHI_VPU_MEM_PD_REG2),
+ VPU_HHI_MEMPD(HHI_MEM_PD_REG0),
+};
+
+static struct meson_ee_pwrc_mem_domain g12a_pwrc_mem_eth[] = {
+ { HHI_MEM_PD_REG0, GENMASK(3, 2) },
+};
+
+static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_vpu[] = {
+ VPU_MEMPD(HHI_VPU_MEM_PD_REG0),
+ VPU_MEMPD(HHI_VPU_MEM_PD_REG1),
+ VPU_MEMPD(HHI_VPU_MEM_PD_REG2),
+ VPU_MEMPD(HHI_VPU_MEM_PD_REG3),
+ { HHI_VPU_MEM_PD_REG4, GENMASK(1, 0) },
+ { HHI_VPU_MEM_PD_REG4, GENMASK(3, 2) },
+ { HHI_VPU_MEM_PD_REG4, GENMASK(5, 4) },
+ { HHI_VPU_MEM_PD_REG4, GENMASK(7, 6) },
+ VPU_HHI_MEMPD(HHI_MEM_PD_REG0),
+};
+
+static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_nna[] = {
+ { HHI_NANOQ_MEM_PD_REG0, 0xff },
+ { HHI_NANOQ_MEM_PD_REG1, 0xff },
+};
+
+static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_usb[] = {
+ { HHI_MEM_PD_REG0, GENMASK(31, 30) },
+};
+
+static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_pcie[] = {
+ { HHI_MEM_PD_REG0, GENMASK(29, 26) },
+};
+
+static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_ge2d[] = {
+ { HHI_MEM_PD_REG0, GENMASK(25, 18) },
+};
+
+static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_audio[] = {
+ { HHI_MEM_PD_REG0, GENMASK(5, 4) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(1, 0) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(3, 2) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(5, 4) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(7, 6) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(13, 12) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(15, 14) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(17, 16) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(19, 18) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(21, 20) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(23, 22) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(25, 24) },
+ { HHI_AUDIO_MEM_PD_REG0, GENMASK(27, 26) },
+};
+
+#define VPU_PD(__name, __resets, __clks, __top_pd, __mem, __get_power) \
+ { \
+ .name = __name, \
+ .reset_names_count = ARRAY_SIZE(__resets), \
+ .reset_names = __resets, \
+ .clk_names_count = ARRAY_SIZE(__clks), \
+ .clk_names = __clks, \
+ .top_pd = __top_pd, \
+ .mem_pd_count = ARRAY_SIZE(__mem), \
+ .mem_pd = __mem, \
+ .get_power = __get_power, \
+ }
+
+#define TOP_PD(__name, __top_pd, __mem) \
+ { \
+ .name = __name, \
+ .top_pd = __top_pd, \
+ .mem_pd_count = ARRAY_SIZE(__mem), \
+ .mem_pd = __mem, \
+ }
+
+#define MEM_PD(__name, __mem) \
+ TOP_PD(__name, NULL, __mem)
+
+static bool pwrc_vpu_get_power(struct meson_ee_pwrc_domain *pwrc_domain);
+
+static struct meson_ee_pwrc_domain_desc g12a_pwrc_domains[] = {
+ [PWRC_G12A_VPU_ID] = VPU_PD("VPU", g12a_pwrc_vpu_resets,
+ g12a_pwrc_vpu_clks, &g12a_pwrc_vpu,
+ g12a_pwrc_mem_vpu,
+ pwrc_vpu_get_power),
+ [PWRC_G12A_ETH_ID] = MEM_PD("ETH", g12a_pwrc_mem_eth),
+};
+
+static struct meson_ee_pwrc_domain_desc sm1_pwrc_domains[] = {
+ [PWRC_SM1_VPU_ID] = VPU_PD("VPU", g12a_pwrc_vpu_resets,
+ g12a_pwrc_vpu_clks, &sm1_pwrc_vpu,
+ sm1_pwrc_mem_vpu,
+ pwrc_vpu_get_power),
+ [PWRC_SM1_NNA_ID] = TOP_PD("NNA", &sm1_pwrc_nna, sm1_pwrc_mem_nna),
+ [PWRC_SM1_USB_ID] = TOP_PD("USB", &sm1_pwrc_usb, sm1_pwrc_mem_usb),
+ [PWRC_SM1_PCIE_ID] = TOP_PD("PCI", &sm1_pwrc_pci, sm1_pwrc_mem_pcie),
+ [PWRC_SM1_GE2D_ID] = TOP_PD("GE2D", &sm1_pwrc_ge2d, sm1_pwrc_mem_ge2d),
+ [PWRC_SM1_AUDIO_ID] = MEM_PD("AUDIO", sm1_pwrc_mem_audio),
+ [PWRC_SM1_ETH_ID] = MEM_PD("ETH", g12a_pwrc_mem_eth),
+};
+
+struct meson_ee_pwrc_domain {
+ struct generic_pm_domain base;
+ bool enabled;
+ struct meson_ee_pwrc *pwrc;
+ struct meson_ee_pwrc_domain_desc desc;
+ struct clk **clks;
+ int num_clks;
+ struct reset_control **rstc;
+ int num_rstc;
+};
+
+struct meson_ee_pwrc {
+ struct regmap *regmap_ao;
+ struct regmap *regmap_hhi;
+ struct meson_ee_pwrc_domain *domains;
+ struct genpd_onecell_data xlate;
+};
+
+static bool pwrc_vpu_get_power(struct meson_ee_pwrc_domain *pwrc_domain)
+{
+ u32 reg;
+
+ regmap_read(pwrc_domain->pwrc->regmap_ao,
+ pwrc_domain->desc.top_pd->sleep_reg, ®);
+
+ return (reg & pwrc_domain->desc.top_pd->sleep_mask);
+}
+
+static int meson_ee_reset_assert(struct meson_ee_pwrc_domain *pwrc_domain)
+{
+ int i, ret;
+
+ for (i = 0 ; i < pwrc_domain->num_rstc ; ++i) {
+ ret = reset_control_assert(pwrc_domain->rstc[i]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int meson_ee_reset_deassert(struct meson_ee_pwrc_domain *pwrc_domain)
+{
+ int i, ret;
+
+ for (i = 0 ; i < pwrc_domain->num_rstc ; ++i) {
+ ret = reset_control_deassert(pwrc_domain->rstc[i]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int meson_ee_clk_disable(struct meson_ee_pwrc_domain *pwrc_domain)
+{
+ int i;
+
+ for (i = 0 ; i < pwrc_domain->num_clks ; ++i)
+ clk_disable(pwrc_domain->clks[i]);
+
+ for (i = 0 ; i < pwrc_domain->num_clks ; ++i)
+ clk_unprepare(pwrc_domain->clks[i]);
+
+ return 0;
+}
+
+static int meson_ee_clk_enable(struct meson_ee_pwrc_domain *pwrc_domain)
+{
+ int i, ret;
+
+ for (i = 0 ; i < pwrc_domain->num_clks ; ++i) {
+ ret = clk_prepare(pwrc_domain->clks[i]);
+ if (ret)
+ goto fail_prepare;
+ }
+
+ for (i = 0 ; i < pwrc_domain->num_clks ; ++i) {
+ ret = clk_enable(pwrc_domain->clks[i]);
+ if (ret)
+ goto fail_enable;
+ }
+
+ return 0;
+
+fail_enable:
+ while (--i)
+ clk_disable(pwrc_domain->clks[i]);
+
+ /* Unprepare all clocks */
+ i = pwrc_domain->num_clks;
+
+fail_prepare:
+ while (--i)
+ clk_unprepare(pwrc_domain->clks[i]);
+
+ return ret;
+}
+
+static int meson_ee_pwrc_off(struct generic_pm_domain *domain)
+{
+ struct meson_ee_pwrc_domain *pwrc_domain =
+ container_of(domain, struct meson_ee_pwrc_domain, base);
+ int i;
+
+ if (pwrc_domain->desc.top_pd)
+ regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
+ pwrc_domain->desc.top_pd->sleep_reg,
+ pwrc_domain->desc.top_pd->sleep_mask,
+ pwrc_domain->desc.top_pd->sleep_mask);
+ udelay(20);
+
+ for (i = 0 ; i < pwrc_domain->desc.mem_pd_count ; ++i)
+ regmap_update_bits(pwrc_domain->pwrc->regmap_hhi,
+ pwrc_domain->desc.mem_pd[i].reg,
+ pwrc_domain->desc.mem_pd[i].mask,
+ pwrc_domain->desc.mem_pd[i].mask);
+
+ udelay(20);
+
+ if (pwrc_domain->desc.top_pd)
+ regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
+ pwrc_domain->desc.top_pd->iso_reg,
+ pwrc_domain->desc.top_pd->iso_mask,
+ pwrc_domain->desc.top_pd->iso_mask);
+
+ if (pwrc_domain->num_clks) {
+ msleep(20);
+ meson_ee_clk_disable(pwrc_domain);
+ }
+
+ return 0;
+}
+
+static int meson_ee_pwrc_on(struct generic_pm_domain *domain)
+{
+ struct meson_ee_pwrc_domain *pwrc_domain =
+ container_of(domain, struct meson_ee_pwrc_domain, base);
+ int i, ret;
+
+ if (pwrc_domain->desc.top_pd)
+ regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
+ pwrc_domain->desc.top_pd->sleep_reg,
+ pwrc_domain->desc.top_pd->sleep_mask, 0);
+ udelay(20);
+
+ for (i = 0 ; i < pwrc_domain->desc.mem_pd_count ; ++i)
+ regmap_update_bits(pwrc_domain->pwrc->regmap_hhi,
+ pwrc_domain->desc.mem_pd[i].reg,
+ pwrc_domain->desc.mem_pd[i].mask, 0);
+
+ udelay(20);
+
+ ret = meson_ee_reset_assert(pwrc_domain);
+ if (ret)
+ return ret;
+
+ if (pwrc_domain->desc.top_pd)
+ regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
+ pwrc_domain->desc.top_pd->iso_reg,
+ pwrc_domain->desc.top_pd->iso_mask, 0);
+
+ ret = meson_ee_reset_deassert(pwrc_domain);
+ if (ret)
+ return ret;
+
+ return meson_ee_clk_enable(pwrc_domain);
+}
+
+static int meson_ee_pwrc_init_domain(struct platform_device *pdev,
+ struct meson_ee_pwrc *sm1_pwrc,
+ struct meson_ee_pwrc_domain *dom)
+{
+ dom->pwrc = sm1_pwrc;
+ dom->num_rstc = dom->desc.reset_names_count;
+ dom->num_clks = dom->desc.clk_names_count;
+
+ if (dom->num_rstc) {
+ int rst;
+
+ dom->rstc = devm_kcalloc(&pdev->dev, dom->num_rstc,
+ sizeof(struct reset_control *), GFP_KERNEL);
+ if (!dom->rstc)
+ return -ENOMEM;
+
+ for (rst = 0 ; rst < dom->num_rstc ; ++rst) {
+ dom->rstc[rst] = devm_reset_control_get_exclusive(
+ &pdev->dev,
+ dom->desc.reset_names[rst]);
+ if (IS_ERR(dom->rstc[rst]))
+ return PTR_ERR(dom->rstc[rst]);
+ }
+ }
+
+ if (dom->num_clks) {
+ int clk;
+
+ dom->clks = devm_kcalloc(&pdev->dev, dom->num_clks,
+ sizeof(struct clk *), GFP_KERNEL);
+ if (!dom->clks)
+ return -ENOMEM;
+
+ for (clk = 0 ; clk < dom->num_clks ; ++clk) {
+ dom->clks[clk] = devm_clk_get(&pdev->dev,
+ dom->desc.clk_names[clk]);
+ if (IS_ERR(dom->clks[clk]))
+ return PTR_ERR(dom->clks[clk]);
+ }
+ }
+
+ dom->base.name = dom->desc.name;
+ dom->base.power_on = meson_ee_pwrc_on;
+ dom->base.power_off = meson_ee_pwrc_off;
+
+ if (dom->desc.get_power) {
+ bool powered_off = dom->desc.get_power(dom);
+ pm_genpd_init(&dom->base, &pm_domain_always_on_gov,
+ powered_off);
+ } else
+ pm_genpd_init(&dom->base, NULL, true);
+
+ return 0;
+}
+
+static int meson_ee_pwrc_probe(struct platform_device *pdev)
+{
+ const struct meson_ee_pwrc_domain_data *match;
+ struct regmap *regmap_ao, *regmap_hhi;
+ struct meson_ee_pwrc *sm1_pwrc;
+ int i, ret;
+
+ match = of_device_get_match_data(&pdev->dev);
+ if (!match) {
+ dev_err(&pdev->dev, "failed to get match data\n");
+ return -ENODEV;
+ }
+
+ sm1_pwrc = devm_kzalloc(&pdev->dev, sizeof(*sm1_pwrc), GFP_KERNEL);
+ if (!sm1_pwrc)
+ return -ENOMEM;
+
+ sm1_pwrc->xlate.domains =
+ devm_kcalloc(&pdev->dev,
+ match->count,
+ sizeof(*sm1_pwrc->xlate.domains),
+ GFP_KERNEL);
+ if (!sm1_pwrc->xlate.domains)
+ return -ENOMEM;
+
+ sm1_pwrc->domains =
+ devm_kcalloc(&pdev->dev,
+ match->count,
+ sizeof(*sm1_pwrc->domains),
+ GFP_KERNEL);
+ if (!sm1_pwrc->domains)
+ return -ENOMEM;
+
+ sm1_pwrc->xlate.num_domains = match->count;
+
+ regmap_hhi = syscon_node_to_regmap(of_get_parent(pdev->dev.of_node));
+ if (IS_ERR(regmap_hhi)) {
+ dev_err(&pdev->dev, "failed to get HHI regmap\n");
+ return PTR_ERR(regmap_hhi);
+ }
+
+ regmap_ao = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+ "amlogic,ao-sysctrl");
+ if (IS_ERR(regmap_ao)) {
+ dev_err(&pdev->dev, "failed to get AO regmap\n");
+ return PTR_ERR(regmap_ao);
+ }
+
+ sm1_pwrc->regmap_ao = regmap_ao;
+ sm1_pwrc->regmap_hhi = regmap_hhi;
+
+ platform_set_drvdata(pdev, sm1_pwrc);
+
+ for (i = 0 ; i < match->count ; ++i) {
+ struct meson_ee_pwrc_domain *dom = &sm1_pwrc->domains[i];
+
+ memcpy(&dom->desc, &match->domains[i], sizeof(dom->desc));
+
+ ret = meson_ee_pwrc_init_domain(pdev, sm1_pwrc, dom);
+ if (ret)
+ return ret;
+
+ sm1_pwrc->xlate.domains[i] = &dom->base;
+ }
+
+ of_genpd_add_provider_onecell(pdev->dev.of_node, &sm1_pwrc->xlate);
+
+ return 0;
+}
+
+static struct meson_ee_pwrc_domain_data meson_ee_g12a_pwrc_data = {
+ .count = ARRAY_SIZE(g12a_pwrc_domains),
+ .domains = g12a_pwrc_domains,
+};
+
+static struct meson_ee_pwrc_domain_data meson_ee_sm1_pwrc_data = {
+ .count = ARRAY_SIZE(sm1_pwrc_domains),
+ .domains = sm1_pwrc_domains,
+};
+
+static const struct of_device_id meson_ee_pwrc_match_table[] = {
+ {
+ .compatible = "amlogic,meson-g12a-pwrc",
+ .data = &meson_ee_g12a_pwrc_data,
+ },
+ {
+ .compatible = "amlogic,meson-sm1-pwrc",
+ .data = &meson_ee_sm1_pwrc_data,
+ },
+ { /* sentinel */ }
+};
+
+static struct platform_driver meson_ee_pwrc_driver = {
+ .probe = meson_ee_pwrc_probe,
+ .driver = {
+ .name = "meson_ee_pwrc",
+ .of_match_table = meson_ee_pwrc_match_table,
+ },
+};
+builtin_platform_driver(meson_ee_pwrc_driver);
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 3/5] arm64: meson-g12: add Everything-Else power domain controller
From: Neil Armstrong @ 2019-08-21 11:41 UTC (permalink / raw)
To: khilman, ulf.hansson
Cc: linux-amlogic, linux-pm, linux-kernel, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190821114121.10430-1-narmstrong@baylibre.com>
Replace the VPU-centric power domain controller by the generic system-wide
Everything-Else power domain controller and setup the right power-domains
properties on the VPU, Ethernet & USB nodes.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../boot/dts/amlogic/meson-g12-common.dtsi | 92 ++++++++++---------
arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 9 ++
arch/arm64/boot/dts/amlogic/meson-g12b.dtsi | 9 ++
arch/arm64/boot/dts/amlogic/meson-sm1.dtsi | 15 ++-
4 files changed, 77 insertions(+), 48 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index a921d6334e5b..8baa6318f180 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -1426,6 +1426,53 @@
clocks = <&xtal>;
clock-names = "xtal";
};
+
+ pwrc: power-controller {
+ compatible = "amlogic,meson-g12a-pwrc";
+ #power-domain-cells = <1>;
+ amlogic,ao-sysctrl = <&rti>;
+ resets = <&reset RESET_VIU>,
+ <&reset RESET_VENC>,
+ <&reset RESET_VCBUS>,
+ <&reset RESET_BT656>,
+ <&reset RESET_RDMA>,
+ <&reset RESET_VENCI>,
+ <&reset RESET_VENCP>,
+ <&reset RESET_VDAC>,
+ <&reset RESET_VDI6>,
+ <&reset RESET_VENCL>,
+ <&reset RESET_VID_LOCK>;
+ reset-names = "viu", "venc", "vcbus", "bt656",
+ "rdma", "venci", "vencp", "vdac",
+ "vdi6", "vencl", "vid_lock";
+ clocks = <&clkc CLKID_VPU>,
+ <&clkc CLKID_VAPB>;
+ clock-names = "vpu", "vapb";
+ /*
+ * VPU clocking is provided by two identical clock paths
+ * VPU_0 and VPU_1 muxed to a single clock by a glitch
+ * free mux to safely change frequency while running.
+ * Same for VAPB but with a final gate after the glitch free mux.
+ */
+ assigned-clocks = <&clkc CLKID_VPU_0_SEL>,
+ <&clkc CLKID_VPU_0>,
+ <&clkc CLKID_VPU>, /* Glitch free mux */
+ <&clkc CLKID_VAPB_0_SEL>,
+ <&clkc CLKID_VAPB_0>,
+ <&clkc CLKID_VAPB_SEL>; /* Glitch free mux */
+ assigned-clock-parents = <&clkc CLKID_FCLK_DIV3>,
+ <0>, /* Do Nothing */
+ <&clkc CLKID_VPU_0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <0>, /* Do Nothing */
+ <&clkc CLKID_VAPB_0>;
+ assigned-clock-rates = <0>, /* Do Nothing */
+ <666666666>,
+ <0>, /* Do Nothing */
+ <0>, /* Do Nothing */
+ <250000000>,
+ <0>; /* Do Nothing */
+ };
};
};
@@ -1773,50 +1820,6 @@
clock-names = "xtal", "mpeg-clk";
};
- pwrc_vpu: power-controller-vpu {
- compatible = "amlogic,meson-g12a-pwrc-vpu";
- #power-domain-cells = <0>;
- amlogic,hhi-sysctrl = <&hhi>;
- resets = <&reset RESET_VIU>,
- <&reset RESET_VENC>,
- <&reset RESET_VCBUS>,
- <&reset RESET_BT656>,
- <&reset RESET_RDMA>,
- <&reset RESET_VENCI>,
- <&reset RESET_VENCP>,
- <&reset RESET_VDAC>,
- <&reset RESET_VDI6>,
- <&reset RESET_VENCL>,
- <&reset RESET_VID_LOCK>;
- clocks = <&clkc CLKID_VPU>,
- <&clkc CLKID_VAPB>;
- clock-names = "vpu", "vapb";
- /*
- * VPU clocking is provided by two identical clock paths
- * VPU_0 and VPU_1 muxed to a single clock by a glitch
- * free mux to safely change frequency while running.
- * Same for VAPB but with a final gate after the glitch free mux.
- */
- assigned-clocks = <&clkc CLKID_VPU_0_SEL>,
- <&clkc CLKID_VPU_0>,
- <&clkc CLKID_VPU>, /* Glitch free mux */
- <&clkc CLKID_VAPB_0_SEL>,
- <&clkc CLKID_VAPB_0>,
- <&clkc CLKID_VAPB_SEL>; /* Glitch free mux */
- assigned-clock-parents = <&clkc CLKID_FCLK_DIV3>,
- <0>, /* Do Nothing */
- <&clkc CLKID_VPU_0>,
- <&clkc CLKID_FCLK_DIV4>,
- <0>, /* Do Nothing */
- <&clkc CLKID_VAPB_0>;
- assigned-clock-rates = <0>, /* Do Nothing */
- <666666666>,
- <0>, /* Do Nothing */
- <0>, /* Do Nothing */
- <250000000>,
- <0>; /* Do Nothing */
- };
-
ao_pinctrl: pinctrl@14 {
compatible = "amlogic,meson-g12a-aobus-pinctrl";
#address-cells = <2>;
@@ -2169,7 +2172,6 @@
#address-cells = <1>;
#size-cells = <0>;
amlogic,canvas = <&canvas>;
- power-domains = <&pwrc_vpu>;
/* CVBS VDAC output port */
cvbs_vdac_port: port@0 {
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
index 733a9d46fc4b..eb5d177d7a99 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
@@ -4,6 +4,7 @@
*/
#include "meson-g12-common.dtsi"
+#include <dt-bindings/power/meson-g12a-power.h>
/ {
compatible = "amlogic,g12a";
@@ -110,6 +111,14 @@
};
};
+ðmac {
+ power-domains = <&pwrc PWRC_G12A_ETH_ID>;
+};
+
+&vpu {
+ power-domains = <&pwrc PWRC_G12A_VPU_ID>;
+};
+
&sd_emmc_a {
amlogic,dram-access-quirk;
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi
index d5edbc1a1991..5628ccd54531 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b.dtsi
@@ -5,6 +5,7 @@
*/
#include "meson-g12-common.dtsi"
+#include <dt-bindings/power/meson-g12a-power.h>
/ {
compatible = "amlogic,g12b";
@@ -101,6 +102,14 @@
compatible = "amlogic,g12b-clkc";
};
+ðmac {
+ power-domains = <&pwrc PWRC_G12A_ETH_ID>;
+};
+
+&vpu {
+ power-domains = <&pwrc PWRC_G12A_VPU_ID>;
+};
+
&sd_emmc_a {
amlogic,dram-access-quirk;
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi b/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi
index e902d4f9165f..37064d7f66c1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi
@@ -5,6 +5,7 @@
*/
#include "meson-g12-common.dtsi"
+#include <dt-bindings/power/meson-sm1-power.h>
/ {
compatible = "amlogic,sm1";
@@ -59,10 +60,18 @@
compatible = "amlogic,meson-sm1-clk-measure";
};
-&pwrc_vpu {
- status = "disabled";
+ðmac {
+ power-domains = <&pwrc PWRC_SM1_ETH_ID>;
+};
+
+&pwrc {
+ compatible = "amlogic,meson-sm1-pwrc";
};
&vpu {
- status = "disabled";
+ power-domains = <&pwrc PWRC_SM1_VPU_ID>;
+};
+
+&usb {
+ power-domains = <&pwrc PWRC_SM1_USB_ID>;
};
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 4/5] arm64: dts: meson-sm1-sei610: add HDMI display support
From: Neil Armstrong @ 2019-08-21 11:41 UTC (permalink / raw)
To: khilman, ulf.hansson
Cc: linux-amlogic, linux-pm, linux-kernel, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190821114121.10430-1-narmstrong@baylibre.com>
Update compatible of the pwc-vpu node and add the HDMI support nodes
for the Amlogic SM1 Based SEI610 Board.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../boot/dts/amlogic/meson-sm1-sei610.dts | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
index 12dab0ba2f26..66bd3bfbaf91 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
@@ -51,6 +51,17 @@
};
};
+ hdmi-connector {
+ compatible = "hdmi-connector";
+ type = "a";
+
+ port {
+ hdmi_connector_in: endpoint {
+ remote-endpoint = <&hdmi_tx_tmds_out>;
+ };
+ };
+ };
+
leds {
compatible = "gpio-leds";
@@ -177,6 +188,18 @@
phy-mode = "rmii";
};
+&hdmi_tx {
+ status = "okay";
+ pinctrl-0 = <&hdmitx_hpd_pins>, <&hdmitx_ddc_pins>;
+ pinctrl-names = "default";
+};
+
+&hdmi_tx_tmds_port {
+ hdmi_tx_tmds_out: endpoint {
+ remote-endpoint = <&hdmi_connector_in>;
+ };
+};
+
&i2c3 {
status = "okay";
pinctrl-0 = <&i2c3_sda_a_pins>, <&i2c3_sck_a_pins>;
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/5] arm64: meson: add support for SM1 Power Domains
From: Neil Armstrong @ 2019-08-21 11:41 UTC (permalink / raw)
To: khilman, ulf.hansson
Cc: linux-amlogic, linux-pm, linux-kernel, linux-arm-kernel,
Neil Armstrong
This patchset introduces a new "Everything-Else Power Domain Controller"
designed to handle all the different non-Always On peripherals like :
- VPU
- Ethernet Memories
- USB, PCIe, Audio, NNA on SM1
The current "gx-vpu-pwrc" process has been integrated to support the VPU
and the other power domains in a single driver.
Support for SoC domains has been made generic and easily extendable.
In order to restart from clean architecture :
- the PWRC node has been moved into the HHI simple-mfd, this suits much
better than beeing in the AO RTI simple-mfd
- a brand new yaml bindings schemas has been written
- reset-names has been added to clarify which resets are needed, so we can
dispatch them to domains
For G12A, the PWRC now offers support for the ethmac memory power domain.
For SM1, it also offers support for PCIe, USB, NNA, ethmac and Audio power
domains.
The DOS domains has been excluded for now, but can be added very easily.
GX hasn't been integrated for now, but it would follow the same scheme
as G12A support.
Neil Armstrong (5):
dt-bindings: power: add Amlogic Everything-Else power domains bindings
soc: amlogic: Add support for Everything-Else power domains controller
arm64: meson-g12: add Everything-Else power domain controller
arm64: dts: meson-sm1-sei610: add HDMI display support
arm64: dts: meson-sm1-sei610: add USB support
.../bindings/power/amlogic,meson-ee-pwrc.yaml | 93 +++
.../boot/dts/amlogic/meson-g12-common.dtsi | 92 +--
arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 9 +
arch/arm64/boot/dts/amlogic/meson-g12b.dtsi | 9 +
.../boot/dts/amlogic/meson-sm1-sei610.dts | 28 +
arch/arm64/boot/dts/amlogic/meson-sm1.dtsi | 15 +-
drivers/soc/amlogic/Kconfig | 11 +
drivers/soc/amlogic/Makefile | 1 +
drivers/soc/amlogic/meson-ee-pwrc.c | 560 ++++++++++++++++++
include/dt-bindings/power/meson-g12a-power.h | 13 +
include/dt-bindings/power/meson-sm1-power.h | 18 +
11 files changed, 801 insertions(+), 48 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-ee-pwrc.yaml
create mode 100644 drivers/soc/amlogic/meson-ee-pwrc.c
create mode 100644 include/dt-bindings/power/meson-g12a-power.h
create mode 100644 include/dt-bindings/power/meson-sm1-power.h
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V3, 2/2] media: i2c: Add Omnivision OV02A10 camera sensor driver
From: Sakari Ailus @ 2019-08-21 11:05 UTC (permalink / raw)
To: Tomasz Figa
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
louis.kuo, sj.huang, robh+dt, linux-mediatek, dongchun.zhu,
matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20190821103038.GA148543@chromium.org>
Hi Tomasz,
On Wed, Aug 21, 2019 at 07:30:38PM +0900, Tomasz Figa wrote:
...
> > +
> > +/*
> > + * xvclk 24Mhz
>
> This seems to assume 24MHz, but the driver allows a range in probe. Is that
> correct?
I think it'd be better to check for an exact frequency: this is board
specific and its exact value is known.
...
> > +static int __ov02a10_power_on(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + struct device *dev = &client->dev;
> > + int ret;
> > +
> > + ret = clk_prepare_enable(ov02a10->xvclk);
> > + if (ret < 0) {
> > + dev_err(dev, "Failed to enable xvclk\n");
> > + return ret;
> > + }
>
> Is it really correct to enable the clock before the regulators?
>
> According to the datasheet, it should be:
> - PD pin HIGH,
> - nRST pin LOW,
> - DVDDIO and AVDD28 power up and stabilize,
> - clock enabled,
> - min 5 ms delay,
> - PD pin LOW,
> - min 4 ms delay,
> - nRST pin HIGH,
> - min 5 ms delay,
> - I2C interface ready.
>
> > +
> > + /* Note: set 0 is high, set 1 is low */
>
> Why is that? If there is some inverter on the way that should be handled
> outside of this driver. (GPIO DT bindings have flags for this purpose.
>
> If the pins are nRESET and nPOWERDOWN in the hardware datasheet, we should
> call them like this in the driver too (+/- the lowercase and underscore
> convention).
>
> According to the datasheet, the reset pin is called RST and inverted, so we should
> call it n_rst, but the powerdown signal, called PD, is not inverted, so pd
> would be the right name.
For what it's worth sensors generally have xshutdown (or reset) pin that is
active high. Looking at the code, it is not the case here. It's a bit odd
since the usual arrangement saves power when the camera is not in use; it's
not a lot but still. Oh well.
...
> > +static struct i2c_driver ov02a10_i2c_driver = {
> > + .driver = {
> > + .name = "ov02a10",
> > + .pm = &ov02a10_pm_ops,
> > + .of_match_table = ov02a10_of_match,
>
> Please use of_match_ptr() wrapper.
Not really needed; the driver does expect regulators, GPIOs etc., but by
leaving out of_match_ptr(), the driver will also probe on ACPI based
systems.
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3] arm64: implement KPROBES_ON_FTRACE
From: Jisheng Zhang @ 2019-08-21 10:46 UTC (permalink / raw)
To: Catalin Marinas, Jonathan Corbet, Will Deacon, Thomas Gleixner,
Masami Hiramatsu, Naveen N. Rao, Peter Zijlstra
Cc: Ingo Molnar, Steven Rostedt, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org
KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
eliminates the need for a trap, as well as the need to emulate or
single-step instructions.
Tested on berlin arm64 platform.
~ # mount -t debugfs debugfs /sys/kernel/debug/
~ # cd /sys/kernel/debug/
/sys/kernel/debug # echo 'p _do_fork' > tracing/kprobe_events
before the patch:
/sys/kernel/debug # cat kprobes/list
ffffff801009fe28 k _do_fork+0x0 [DISABLED]
after the patch:
/sys/kernel/debug # cat kprobes/list
ffffff801009ff54 k _do_fork+0x4 [DISABLED][FTRACE]
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
eliminates the need for a trap, as well as the need to emulate or
single-step instructions.
Applied after arm64 FTRACE_WITH_REGS:
http://lists.infradead.org/pipermail/linux-arm-kernel/2019-August/674404.html
Changes since v2:
- remove patch1, make it a single cleanup patch
- remove "This patch" in the change log
- implement arm64's kprobe_lookup_name() and arch_kprobe_on_func_entry instead
patching the common kprobes code
Changes since v1:
- make the kprobes/x86: use instruction_pointer and instruction_pointer_set
as patch1
- add Masami's ACK to patch1
- add some description about KPROBES_ON_FTRACE and why we need it on
arm64
- correct the log before the patch
- remove the consolidation patch, make it as TODO
- only adjust kprobe's addr when KPROBE_FLAG_FTRACE is set
- if KPROBES_ON_FTRACE, ftrace_call_adjust() the kprobe's addr before
calling ftrace_location()
- update the kprobes-on-ftrace/arch-support.txt in doc
.../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/probes/Makefile | 1 +
arch/arm64/kernel/probes/ftrace.c | 60 +++++++++++++++++++
arch/arm64/kernel/probes/kprobes.c | 23 +++++++
5 files changed, 86 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kernel/probes/ftrace.c
diff --git a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
index 68f266944d5f..e8358a38981c 100644
--- a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
+++ b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
@@ -9,7 +9,7 @@
| alpha: | TODO |
| arc: | TODO |
| arm: | TODO |
- | arm64: | TODO |
+ | arm64: | ok |
| c6x: | TODO |
| csky: | TODO |
| h8300: | TODO |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 663392d1eae2..928700f15e23 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -167,6 +167,7 @@ config ARM64
select HAVE_STACKPROTECTOR
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_KPROBES
+ select HAVE_KPROBES_ON_FTRACE
select HAVE_KRETPROBES
select HAVE_GENERIC_VDSO
select IOMMU_DMA if IOMMU_SUPPORT
diff --git a/arch/arm64/kernel/probes/Makefile b/arch/arm64/kernel/probes/Makefile
index 8e4be92e25b1..4020cfc66564 100644
--- a/arch/arm64/kernel/probes/Makefile
+++ b/arch/arm64/kernel/probes/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o \
simulate-insn.o
obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o \
simulate-insn.o
+obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
diff --git a/arch/arm64/kernel/probes/ftrace.c b/arch/arm64/kernel/probes/ftrace.c
new file mode 100644
index 000000000000..1f0c09d02bb8
--- /dev/null
+++ b/arch/arm64/kernel/probes/ftrace.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Dynamic Ftrace based Kprobes Optimization
+ *
+ * Copyright (C) Hitachi Ltd., 2012
+ * Copyright (C) 2019 Jisheng Zhang <jszhang@kernel.org>
+ * Synaptics Incorporated
+ */
+
+#include <linux/kprobes.h>
+
+/* Ftrace callback handler for kprobes -- called under preepmt disabed */
+void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *ops, struct pt_regs *regs)
+{
+ struct kprobe *p;
+ struct kprobe_ctlblk *kcb;
+
+ /* Preempt is disabled by ftrace */
+ p = get_kprobe((kprobe_opcode_t *)ip);
+ if (unlikely(!p) || kprobe_disabled(p))
+ return;
+
+ kcb = get_kprobe_ctlblk();
+ if (kprobe_running()) {
+ kprobes_inc_nmissed_count(p);
+ } else {
+ unsigned long orig_ip = instruction_pointer(regs);
+ /* Kprobe handler expects regs->pc = pc + 4 as breakpoint hit */
+ instruction_pointer_set(regs, ip + sizeof(kprobe_opcode_t));
+
+ __this_cpu_write(current_kprobe, p);
+ kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+ if (!p->pre_handler || !p->pre_handler(p, regs)) {
+ /*
+ * Emulate singlestep (and also recover regs->pc)
+ * as if there is a nop
+ */
+ instruction_pointer_set(regs,
+ (unsigned long)p->addr + MCOUNT_INSN_SIZE);
+ if (unlikely(p->post_handler)) {
+ kcb->kprobe_status = KPROBE_HIT_SSDONE;
+ p->post_handler(p, regs, 0);
+ }
+ instruction_pointer_set(regs, orig_ip);
+ }
+ /*
+ * If pre_handler returns !0, it changes regs->pc. We have to
+ * skip emulating post_handler.
+ */
+ __this_cpu_write(current_kprobe, NULL);
+ }
+}
+NOKPROBE_SYMBOL(kprobe_ftrace_handler);
+
+int arch_prepare_kprobe_ftrace(struct kprobe *p)
+{
+ p->ainsn.api.insn = NULL;
+ return 0;
+}
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index c4452827419b..f2bf8c70da79 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -551,6 +551,29 @@ void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
return (void *)orig_ret_address;
}
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
+{
+ unsigned long addr = kallsyms_lookup_name(name);
+ unsigned long faddr;
+
+ /*
+ * with -fpatchable-function-entry=2, the first 4 bytes is the
+ * LR saver, then the actual call insn. So ftrace location is
+ * always on the first 4 bytes offset.
+ */
+ faddr = ftrace_location_range(addr, addr + AARCH64_INSN_SIZE);
+ if (faddr)
+ return (kprobe_opcode_t *)faddr;
+ return (kprobe_opcode_t *)addr;
+}
+
+bool arch_kprobe_on_func_entry(unsigned long offset)
+{
+ return offset <= AARCH64_INSN_SIZE;
+}
+#endif
+
void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs)
{
--
2.23.0.rc1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [V3, 2/2] media: i2c: Add Omnivision OV02A10 camera sensor driver
From: Tomasz Figa @ 2019-08-21 10:30 UTC (permalink / raw)
To: dongchun.zhu
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
louis.kuo, sj.huang, robh+dt, linux-mediatek, sakari.ailus,
matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20190819034331.13098-3-dongchun.zhu@mediatek.com>
Hi Dongchun,
On Mon, Aug 19, 2019 at 11:43:31AM +0800, dongchun.zhu@mediatek.com wrote:
> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
>
> This patch adds a V4L2 sub-device driver for OV02A10 image sensor.
> The OV02A10 is a 1/5" CMOS sensor from Omnivision,
> which supports output format: 10-bit Raw.
> The OV02A10 has a single MIPI lane interface and use the I2C bus
> for control and the CSI-2 bus for data.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 11 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/ov02a10.c | 1018 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 1031 insertions(+)
> create mode 100644 drivers/media/i2c/ov02a10.c
>
Thanks for the patch! Please see my comments inline.
[snip]
> +#define CHIP_ID 0x2509
> +#define OV02A10_REG_CHIP_ID_H 0x02
> +#define OV02A10_REG_CHIP_ID_L 0x03
> +#define OV02A10_ID(_msb, _lsb) ((_msb) << 8 | (_lsb))
> +
> +/* Bit[1] vertical upside down */
> +/* Bit[0] horizontal mirror */
> +#define REG_MIRROR_FLIP_CONTROL 0x3f
> +
> +/* Orientation */
> +#define REG_CONFIG_MIRROR_FLIP 0x03
> +
> +#define REG_PAGE_SWITCH 0xfd
> +#define REG_GLOBAL_EFFECTIVE 0x01
> +#define REG_ENABLE BIT(0)
> +
> +#define REG_SC_CTRL_MODE 0xac
> +#define SC_CTRL_MODE_STANDBY 0x00
> +#define SC_CTRL_MODE_STREAMING 0x01
> +
> +#define OV02A10_REG_EXPOSURE_H 0x03
> +#define OV02A10_REG_EXPOSURE_L 0x04
> +#define OV02A10_EXPOSURE_MIN 4
> +#define OV02A10_EXPOSURE_STEP 1
> +
> +#define OV02A10_REG_VTS_H 0x05
> +#define OV02A10_REG_VTS_L 0x06
> +#define OV02A10_VTS_MAX 0x209f
> +#define OV02A10_VTS_MIN 0x04cf
> +#define OV02A10_BASIC_LINE 1224
> +
> +#define OV02A10_REG_GAIN 0x24
> +#define OV02A10_GAIN_MIN 0x10
> +#define OV02A10_GAIN_MAX 0xf8
> +#define OV02A10_GAIN_STEP 0x01
> +#define OV02A10_GAIN_DEFAULT 0x40
> +
> +#define REG_NULL 0xff
> +
> +#define OV02A10_LANES 1
> +#define OV02A10_BITS_PER_SAMPLE 10
> +
I think there is something wrong with the indentation in the code above.
Please use tabs wherever possible.
[snip]
> +
> +#define to_ov02a10(sd) container_of(sd, struct ov02a10, subdev)
Please use a static inline function for added compile-time type checks.
> +
> +static inline void msleep_range(unsigned int delay_base)
> +{
> + usleep_range(delay_base * 1000, delay_base * 1000 + 500);
> +}
Why not just use msleep()?
> +
> +/* MIPI color bar enable output */
> +static const struct regval ov02a10_test_pattern_enable_regs[] = {
> + {0xfd, 0x01},
> + {0x0d, 0x00},
> + {0xb6, 0x01},
> + {0x01, 0x01},
> + {0xfd, 0x01},
> + {0xac, 0x01},
> + {REG_NULL, 0x00}
> +};
> +
> +/* MIPI color bar disable output */
> +static const struct regval ov02a10_test_pattern_disable_regs[] = {
> + {0xfd, 0x01},
> + {0x0d, 0x00},
> + {0xb6, 0x00},
> + {0x01, 0x01},
> + {0xfd, 0x01},
> + {0xac, 0x01},
> + {REG_NULL, 0x00}
> +};
Hmm, only the register 0xb6 seems to here. Could we just set it directly,
without these arrays?
> +
> +/*
> + * xvclk 24Mhz
This seems to assume 24MHz, but the driver allows a range in probe. Is that
correct?
[snip]]
> +/* Write a register */
> +static int ov02a10_write_reg(struct ov02a10 *ov02a10, u8 addr, u8 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + u8 buf[2] = {addr, val};
> + int ret;
> +
> + ret = i2c_master_send(client, buf, 2);
> +
> + if (ret != 2) {
> + dev_err(&client->dev, "%s: error: reg=%x, val=%x\n",
> + __func__, addr, val);
> + return -EIO;
> + }
> +
> + return 0;
> +}
Could this be replaced with i2c_smbus_write_byte_data()?
> +
> +static int ov02a10_write_array(struct ov02a10 *ov02a10,
> + const struct regval *regs)
> +{
> + u32 i;
> + int ret;
> +
> + for (i = 0; regs[i].addr != REG_NULL; i++) {
> + ret = ov02a10_write_reg(ov02a10, regs[i].addr, regs[i].val);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/* Read a register */
> +static int ov02a10_read_reg(struct ov02a10 *ov02a10, u8 reg, u8 *val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + u8 data = reg;
> + struct i2c_msg msg = {
> + .addr = client->addr,
> + .flags = 0,
> + .len = 1,
> + .buf = &data,
> + };
> + int ret;
> +
> + ret = i2c_transfer(client->adapter, &msg, 1);
> + if (ret < 0)
> + goto err_wr;
> +
> + msg.flags = I2C_M_RD;
> + ret = i2c_transfer(client->adapter, &msg, 1);
> + if (ret < 0)
> + goto err_rd;
Could we just have 2 messages in an array and just call i2c_transfer() once
for both write and read?
Or actually it sounds like the i2c_smbus_read_byte_data() helper could work
here.
> +
> + *val = data;
> + return 0;
> +
> +err_rd:
> + dev_err(&client->dev, "i2c_transfer --I2C_M_RD failed\n");
> +err_wr:
> + dev_err(&client->dev, "read error: reg=0x%02x: %d\n", reg, ret);
> + return ret;
> +}
> +
> +static void ov02a10_fill_fmt(const struct ov02a10_mode *mode,
> + struct v4l2_mbus_framefmt *fmt)
> +{
> + fmt->width = mode->width;
> + fmt->height = mode->height;
> + fmt->field = V4L2_FIELD_NONE;
> +}
> +
> +static int ov02a10_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> +
> + mutex_lock(&ov02a10->mutex);
> +
> + if (ov02a10->streaming) {
> + mutex_unlock(&ov02a10->mutex);
> + return -EBUSY;
> + }
> +
> + /* Only one sensor mode supported */
> + mbus_fmt->code = ov02a10->fmt.code;
> + ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> + ov02a10->fmt = fmt->format;
> +
> + mutex_unlock(&ov02a10->mutex);
> +
> + return 0;
> +}
> +
> +static int ov02a10_get_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> +
> + mutex_lock(&ov02a10->mutex);
> +
> + fmt->format = ov02a10->fmt;
> + mbus_fmt->code = ov02a10->fmt.code;
> + ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> +
> + mutex_unlock(&ov02a10->mutex);
> +
> + return 0;
> +}
> +
> +static int ov02a10_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> +
> + if (code->index >= ARRAY_SIZE(supported_modes) || !(code->index))
Hmm, ARRAY_SIZE(supported_modes) is 1 and we don't allow code->index to be
0 either. Is there a code->index value that wouldn't return an error here?
> + return -EINVAL;
> +
> + code->code = ov02a10->fmt.code;
> +
> + return 0;
> +}
> +
> +static int ov02a10_enum_frame_sizes(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + if (fse->index >= ARRAY_SIZE(supported_modes) || !(fse->index))
Same here.
> + return -EINVAL;
> +
> + fse->min_width = supported_modes[fse->index].width;
> + fse->max_width = supported_modes[fse->index].width;
> + fse->max_height = supported_modes[fse->index].height;
> + fse->min_height = supported_modes[fse->index].height;
> +
> + return 0;
> +}
> +
> +static int __ov02a10_power_on(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + struct device *dev = &client->dev;
> + int ret;
> +
> + ret = clk_prepare_enable(ov02a10->xvclk);
> + if (ret < 0) {
> + dev_err(dev, "Failed to enable xvclk\n");
> + return ret;
> + }
Is it really correct to enable the clock before the regulators?
According to the datasheet, it should be:
- PD pin HIGH,
- nRST pin LOW,
- DVDDIO and AVDD28 power up and stabilize,
- clock enabled,
- min 5 ms delay,
- PD pin LOW,
- min 4 ms delay,
- nRST pin HIGH,
- min 5 ms delay,
- I2C interface ready.
> +
> + /* Note: set 0 is high, set 1 is low */
Why is that? If there is some inverter on the way that should be handled
outside of this driver. (GPIO DT bindings have flags for this purpose.
If the pins are nRESET and nPOWERDOWN in the hardware datasheet, we should
call them like this in the driver too (+/- the lowercase and underscore
convention).
According to the datasheet, the reset pin is called RST and inverted, so we should
call it n_rst, but the powerdown signal, called PD, is not inverted, so pd
would be the right name.
> + gpiod_set_value_cansleep(ov02a10->reset_gpio, 1);
> + gpiod_set_value_cansleep(ov02a10->powerdown_gpio, 0);
> +
> + ret = regulator_bulk_enable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
> + if (ret < 0) {
> + dev_err(dev, "Failed to enable regulators\n");
> + goto disable_clk;
> + }
> + msleep_range(7);
> +
> + gpiod_set_value_cansleep(ov02a10->powerdown_gpio, 1);
> + msleep_range(10);
> +
> + gpiod_set_value_cansleep(ov02a10->reset_gpio, 0);
> + msleep_range(10);
> +
> + return 0;
> +
> +disable_clk:
> + clk_disable_unprepare(ov02a10->xvclk);
> +
> + return ret;
> +}
> +
> +static void __ov02a10_power_off(struct ov02a10 *ov02a10)
> +{
> + clk_disable_unprepare(ov02a10->xvclk);
> + gpiod_set_value_cansleep(ov02a10->reset_gpio, 1);
> + gpiod_set_value_cansleep(ov02a10->powerdown_gpio, 1);
> + regulator_bulk_disable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
This also doesn't seem to match my datasheet. The sequence there is:
- nRST goes LOW,
- clock stops,
- PD goes HIGH,
- regulators are powerd down.
> +}
> +
> +static int __ov02a10_start_stream(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + int ret;
> +
> + /* Apply default values of current mode */
> + ret = ov02a10_write_array(ov02a10, ov02a10->cur_mode->reg_list);
> + if (ret)
> + return ret;
> +
> + /* Apply customized values from user */
> + ret = __v4l2_ctrl_handler_setup(ov02a10->subdev.ctrl_handler);
> + if (ret)
> + return ret;
> +
> + /* Set orientation to 180 degree */
> + if (ov02a10->upside_down) {
> + ret = ov02a10_write_reg(ov02a10, REG_MIRROR_FLIP_CONTROL,
> + REG_CONFIG_MIRROR_FLIP);
> + if (ret) {
> + dev_err(&client->dev, "%s failed to set orientation\n",
> + __func__);
> + return ret;
> + }
> + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> + REG_ENABLE);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /* Set stream on register */
> + return ov02a10_write_reg(ov02a10,
> + REG_SC_CTRL_MODE, SC_CTRL_MODE_STREAMING);
> +}
> +
> +static int __ov02a10_stop_stream(struct ov02a10 *ov02a10)
> +{
> + return ov02a10_write_reg(ov02a10,
> + REG_SC_CTRL_MODE, SC_CTRL_MODE_STANDBY);
> +}
> +
> +static int ov02a10_entity_init_cfg(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_pad_config *cfg)
> +{
> + struct v4l2_subdev_format fmt = { 0 };
> +
> + fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> + fmt.format.width = 1600;
> + fmt.format.height = 1200;
Where do these values come from? Should we have some macros for them?
> +
> + ov02a10_set_fmt(subdev, cfg, &fmt);
> +
> + return 0;
> +}
> +
> +static int ov02a10_s_stream(struct v4l2_subdev *sd, int on)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + int ret = 0;
> +
> + mutex_lock(&ov02a10->mutex);
> +
> + if (ov02a10->streaming == on)
> + goto unlock_and_return;
> +
> + if (on) {
> + ret = pm_runtime_get_sync(&client->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(&client->dev);
> + goto unlock_and_return;
> + }
> +
> + ret = __ov02a10_start_stream(ov02a10);
> + if (ret) {
> + __ov02a10_stop_stream(ov02a10);
> + ov02a10->streaming = !on;
> + goto err_rpm_put;
> + }
> + } else {
> + __ov02a10_stop_stream(ov02a10);
> + pm_runtime_put(&client->dev);
> + }
> +
> + ov02a10->streaming = on;
> + mutex_unlock(&ov02a10->mutex);
> +
> + return ret;
> +
> +err_rpm_put:
> + pm_runtime_put(&client->dev);
> +unlock_and_return:
> + mutex_unlock(&ov02a10->mutex);
> +
> + return ret;
> +}
> +
> +static int ov02a10_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> + struct v4l2_mbus_framefmt *try_fmt = v4l2_subdev_get_try_format(sd,
> + fh->pad,
> + 0);
Please separate the initialization from the declaration, because there
isn't just enough space on this line anymore.
> +
> + mutex_lock(&ov02a10->mutex);
> + /* Initialize try_fmt */
> + try_fmt->code = ov02a10->fmt.code;
> + ov02a10_fill_fmt(&supported_modes[0], try_fmt);
> +
> + mutex_unlock(&ov02a10->mutex);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused ov02a10_runtime_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> +
> + return __ov02a10_power_on(ov02a10);
> +}
> +
> +static int __maybe_unused ov02a10_runtime_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> +
> + __ov02a10_power_off(ov02a10);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops ov02a10_pm_ops = {
> + SET_RUNTIME_PM_OPS(ov02a10_runtime_suspend,
> + ov02a10_runtime_resume, NULL)
Don't we need to implement and provide system PM ops too?
> +};
> +
> +static int ov02a10_set_test_pattern(struct ov02a10 *ov02a10, s32 value)
> +{
> + if (value)
> + return ov02a10_write_array(ov02a10,
> + ov02a10_test_pattern_enable_regs);
> +
> + return ov02a10_write_array(ov02a10,
> + ov02a10_test_pattern_disable_regs);
> +}
> +
> +static int ov02a10_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct ov02a10 *ov02a10 = container_of(ctrl->handler,
> + struct ov02a10, ctrl_handler);
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + s64 max_expo;
> + int ret;
> +
> + /* Propagate change of current control to all related controls */
> + if (ctrl->id == V4L2_CID_VBLANK) {
> + /* Update max exposure while meeting expected vblanking */
> + max_expo = ov02a10->cur_mode->height + ctrl->val - 4;
> + __v4l2_ctrl_modify_range(ov02a10->exposure,
> + ov02a10->exposure->minimum, max_expo,
> + ov02a10->exposure->step,
> + ov02a10->exposure->default_value);
> + }
> +
> + /* V4L2 controls values will be applied only when power is already up */
> + if (!pm_runtime_get_if_in_use(&client->dev))
> + return 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_EXPOSURE:
> + ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> + if (ret < 0)
> + return ret;
> + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_EXPOSURE_H,
> + ((ctrl->val >> 8) & 0xFF));
> + if (!ret) {
> + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_EXPOSURE_L,
> + (ctrl->val & 0xFF));
> + if (ret < 0)
> + return ret;
> + }
> + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> + REG_ENABLE);
> + if (ret < 0)
> + return ret;
> + break;
> + case V4L2_CID_ANALOGUE_GAIN:
> + ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> + if (ret < 0)
> + return ret;
> + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_GAIN,
> + (ctrl->val & 0xFF));
> + if (ret < 0)
> + return ret;
> + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> + REG_ENABLE);
> + if (ret < 0)
> + return ret;
> + break;
> + case V4L2_CID_VBLANK:
> + ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> + if (ret < 0)
> + return ret;
> + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_VTS_H,
> + (((ctrl->val +
> + ov02a10->cur_mode->height -
> + OV02A10_BASIC_LINE) >> 8)
> + & 0xFF));
> + if (!ret) {
> + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_VTS_L,
> + ((ctrl->val +
> + ov02a10->cur_mode->height -
> + OV02A10_BASIC_LINE) & 0xFF));
> + if (ret < 0)
> + return ret;
> + }
> + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> + REG_ENABLE);
> + if (ret < 0)
> + return ret;
> + break;
> + case V4L2_CID_TEST_PATTERN:
> + ret = ov02a10_set_test_pattern(ov02a10, ctrl->val);
> + if (ret < 0)
> + return ret;
> + break;
> + default:
> + dev_warn(&client->dev, "%s Unhandled id:0x%x, val:0x%x\n",
> + __func__, ctrl->id, ctrl->val);
> + ret = -EINVAL;
> + break;
We shouldn't need to handle this, as the control framework wouldn't call us
with a control that we didn't register explicitly.
> + };
> +
> + pm_runtime_put(&client->dev);
> +
> + return ret;
> +}
> +
> +static const struct v4l2_subdev_video_ops ov02a10_video_ops = {
> + .s_stream = ov02a10_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops ov02a10_pad_ops = {
> + .init_cfg = ov02a10_entity_init_cfg,
> + .enum_mbus_code = ov02a10_enum_mbus_code,
> + .enum_frame_size = ov02a10_enum_frame_sizes,
> + .get_fmt = ov02a10_get_fmt,
> + .set_fmt = ov02a10_set_fmt,
> +};
> +
> +static const struct v4l2_subdev_ops ov02a10_subdev_ops = {
> + .video = &ov02a10_video_ops,
> + .pad = &ov02a10_pad_ops,
> +};
> +
> +static const struct media_entity_operations ov02a10_subdev_entity_ops = {
> + .link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static const struct v4l2_subdev_internal_ops ov02a10_internal_ops = {
> + .open = ov02a10_open,
> +};
> +
> +static const struct v4l2_ctrl_ops ov02a10_ctrl_ops = {
> + .s_ctrl = ov02a10_set_ctrl,
> +};
> +
> +static int ov02a10_initialize_controls(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + const struct ov02a10_mode *mode;
> + struct v4l2_ctrl_handler *handler;
> + struct v4l2_ctrl *ctrl;
> + u64 exposure_max;
> + u32 pixel_rate, h_blank;
> + int ret;
> +
> + handler = &ov02a10->ctrl_handler;
> + mode = ov02a10->cur_mode;
> + ret = v4l2_ctrl_handler_init(handler, 10);
I can see 6 controls registered below.
> + if (ret)
> + return ret;
> + handler->lock = &ov02a10->mutex;
> +
> + ctrl = v4l2_ctrl_new_int_menu(handler, NULL, V4L2_CID_LINK_FREQ,
> + 0, 0, link_freq_menu_items);
> + if (ctrl)
> + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + pixel_rate = (link_freq_menu_items[0] * 2 * OV02A10_LANES) /
> + OV02A10_BITS_PER_SAMPLE;
> + v4l2_ctrl_new_std(handler, NULL, V4L2_CID_PIXEL_RATE,
> + 0, pixel_rate, 1, pixel_rate);
> +
> + h_blank = mode->hts_def - mode->width;
> + ov02a10->hblank = v4l2_ctrl_new_std(handler, NULL, V4L2_CID_HBLANK,
> + h_blank, h_blank, 1, h_blank);
> + if (ov02a10->hblank)
> + ov02a10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + ov02a10->vblank = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> + V4L2_CID_VBLANK, mode->vts_def -
> + mode->height,
> + OV02A10_VTS_MAX - mode->height, 1,
> + mode->vts_def - mode->height);
> +
> + exposure_max = mode->vts_def - 4;
> + ov02a10->exposure = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> + V4L2_CID_EXPOSURE,
> + OV02A10_EXPOSURE_MIN,
> + exposure_max,
> + OV02A10_EXPOSURE_STEP,
> + mode->exp_def);
> +
> + ov02a10->anal_gain = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> + V4L2_CID_ANALOGUE_GAIN,
> + OV02A10_GAIN_MIN,
> + OV02A10_GAIN_MAX,
> + OV02A10_GAIN_STEP,
> + OV02A10_GAIN_DEFAULT);
> +
> + ov02a10->test_pattern =
> + v4l2_ctrl_new_std_menu_items(handler,
> + &ov02a10_ctrl_ops,
> + V4L2_CID_TEST_PATTERN,
> + ARRAY_SIZE(ov02a10_test_pattern_menu) -
> + 1, 0, 0, ov02a10_test_pattern_menu);
> +
> + if (handler->error) {
> + ret = handler->error;
> + dev_err(&client->dev,
> + "Failed to init controls(%d)\n", ret);
> + goto err_free_handler;
> + }
> +
> + ov02a10->subdev.ctrl_handler = handler;
> +
> + return 0;
> +
> +err_free_handler:
> + v4l2_ctrl_handler_free(handler);
> +
> + return ret;
> +}
> +
> +static int ov02a10_check_sensor_id(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + u16 id;
> + u8 pid = 0;
> + u8 ver = 0;
> + int ret;
> +
> + /* Check sensor revision */
> + ret = ov02a10_read_reg(ov02a10, OV02A10_REG_CHIP_ID_H, &pid);
> + if (ret)
> + return ret;
> +
> + ret = ov02a10_read_reg(ov02a10, OV02A10_REG_CHIP_ID_L, &ver);
> + if (ret)
> + return ret;
> +
> + id = OV02A10_ID(pid, ver);
> + if (id != CHIP_ID) {
> + dev_err(&client->dev, "Unexpected sensor id(%04x)\n", id);
> + return ret;
> + }
> +
> + dev_info(&client->dev, "Detected OV%04X sensor\n", id);
> +
> + return 0;
> +}
> +
> +static int ov02a10_configure_regulators(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + unsigned int i;
> +
> + for (i = 0; i < OV02A10_NUM_SUPPLIES; i++)
> + ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> +
> + return devm_regulator_bulk_get(&client->dev,
> + OV02A10_NUM_SUPPLIES,
> + ov02a10->supplies);
> +}
I think we can just have this directly inside probe.
> +
> +static int ov02a10_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct ov02a10 *ov02a10;
> + u32 rotation;
> + u32 xclk_freq;
> + int ret;
> +
> + ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> + if (!ov02a10)
> + return -ENOMEM;
> +
> + v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> + ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> +
> + /* Optional indication of physical rotation of sensor */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation",
> + &rotation);
> + if (!ret) {
> + switch (rotation) {
> + case 180:
> + ov02a10->upside_down = true;
> + ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> + break;
> + case 0:
> + break;
> + default:
> + dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
> + rotation);
> + }
> + }
> +
> + /* Get system clock (xvclk) */
> + ov02a10->xvclk = devm_clk_get(dev, "xvclk");
> + if (IS_ERR(ov02a10->xvclk)) {
> + dev_err(dev, "Failed to get xvclk\n");
> + return -EINVAL;
> + }
Hmm, it's called eclk in my datasheet.
> +
> + ret = of_property_read_u32(dev->of_node, "clock-frequency", &xclk_freq);
> + if (ret) {
> + dev_err(dev, "Failed to get xclk frequency\n");
> + return ret;
> + }
> +
> + /* External clock must be 24MHz, allow 1% tolerance */
> + if (xclk_freq < 23760000 || xclk_freq > 24240000) {
How do we support a range of frequencies? I don't see the driver calculate
any register values based on this frequency. Are you sure that the register
arrays don't assume one specific frequency?
> + dev_err(dev, "external clock frequency %u is not supported\n",
> + xclk_freq);
> + return -EINVAL;
> + }
> + dev_dbg(dev, "external clock frequency %u\n", xclk_freq);
> +
> + ret = clk_set_rate(ov02a10->xvclk, xclk_freq);
> + if (ret) {
> + dev_err(dev, "Failed to set xvclk frequency (24MHz)\n");
> + return ret;
> + }
> +
> + ov02a10->powerdown_gpio = devm_gpiod_get(dev, "powerdown",
> + GPIOD_OUT_LOW);
Hmm, shouldn't this be HIGH? At least the datasheet has it so for the
powered down state.
> + if (IS_ERR(ov02a10->powerdown_gpio)) {
> + dev_err(dev, "Failed to get powerdown-gpios\n");
> + return -EINVAL;
> + }
> +
> + ov02a10->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
Also LOW here, similarly to the above.
> + if (IS_ERR(ov02a10->reset_gpio)) {
> + dev_err(dev, "Failed to get reset-gpios\n");
> + return -EINVAL;
> + }
> +
> + ret = ov02a10_configure_regulators(ov02a10);
> + if (ret) {
> + dev_err(dev, "Failed to get power regulators\n");
> + return ret;
> + }
> +
> + mutex_init(&ov02a10->mutex);
> + ov02a10->cur_mode = &supported_modes[0];
> + ret = ov02a10_initialize_controls(ov02a10);
> + if (ret) {
> + dev_err(dev, "Failed to initialize controls\n");
> + goto err_destroy_mutex;
> + }
> +
> + ret = __ov02a10_power_on(ov02a10);
> + if (ret)
> + goto err_free_handler;
> +
> + ret = ov02a10_check_sensor_id(ov02a10);
> + if (ret)
> + goto err_power_off;
> +
> + ov02a10->subdev.internal_ops = &ov02a10_internal_ops;
> + ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> + ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> + ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> + ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> + if (ret < 0) {
> + dev_err(dev, "failed to init entity pads: %d", ret);
> + goto err_power_off;
> + }
> +
> + ret = v4l2_async_register_subdev(&ov02a10->subdev);
> + if (ret) {
> + dev_err(dev, "failed to register V4L2 subdev: %d",
> + ret);
> + goto err_clean_entity;
> + }
> +
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + pm_runtime_idle(dev);
> +
> + dev_info(dev, "ov02a10 probe --\n");
Please remove this.
> + return 0;
> +
> +err_clean_entity:
> + media_entity_cleanup(&ov02a10->subdev.entity);
> +err_power_off:
> + __ov02a10_power_off(ov02a10);
> +err_free_handler:
> + v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> +err_destroy_mutex:
> + mutex_destroy(&ov02a10->mutex);
> +
> + return ret;
> +}
> +
> +static int ov02a10_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> + v4l2_ctrl_handler_free(sd->ctrl_handler);
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev))
> + __ov02a10_power_off(ov02a10);
> + pm_runtime_set_suspended(&client->dev);
> + mutex_destroy(&ov02a10->mutex);
> +
> + return 0;
> +}
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id ov02a10_of_match[] = {
> + { .compatible = "ovti,ov02a10" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> +#endif
> +
> +static struct i2c_driver ov02a10_i2c_driver = {
> + .driver = {
> + .name = "ov02a10",
> + .pm = &ov02a10_pm_ops,
> + .of_match_table = ov02a10_of_match,
Please use of_match_ptr() wrapper.
Best regards,
Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 4/9] KVM: arm64: Support stolen time reporting via shared structure
From: Steven Price @ 2019-08-21 10:27 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon, linux-arm-kernel, kvmarm
Cc: Mark Rutland, kvm, Radim Krčmář, Catalin Marinas,
Suzuki K Pouloze, linux-doc, Russell King, linux-kernel,
James Morse, Paolo Bonzini, Julien Thierry
In-Reply-To: <f6fad4fa-323d-306c-c582-de07464f4d00@kernel.org>
On 19/08/2019 17:40, Marc Zyngier wrote:
> Hi Steven,
>
> On 19/08/2019 15:04, Steven Price wrote:
>> Implement the service call for configuring a shared structure between a
>> VCPU and the hypervisor in which the hypervisor can write the time
>> stolen from the VCPU's execution time by other tasks on the host.
>>
>> The hypervisor allocates memory which is placed at an IPA chosen by user
>> space. The hypervisor then uses WRITE_ONCE() to update the shared
>> structure ensuring single copy atomicity of the 64-bit unsigned value
>> that reports stolen time in nanoseconds.
>>
>> Whenever stolen time is enabled by the guest, the stolen time counter is
>> reset.
>>
>> The stolen time itself is retrieved from the sched_info structure
>> maintained by the Linux scheduler code. We enable SCHEDSTATS when
>> selecting KVM Kconfig to ensure this value is meaningful.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> arch/arm/include/asm/kvm_host.h | 15 +++++++
>> arch/arm64/include/asm/kvm_host.h | 16 ++++++-
>> arch/arm64/kvm/Kconfig | 1 +
>> include/linux/kvm_types.h | 2 +
>> virt/kvm/arm/arm.c | 19 +++++++++
>> virt/kvm/arm/hypercalls.c | 3 ++
>> virt/kvm/arm/pvtime.c | 71 +++++++++++++++++++++++++++++++
>> 7 files changed, 126 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 369b5d2d54bf..14d61a84c270 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -39,6 +39,7 @@
>> KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
>> #define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
>> #define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
>> +#define KVM_REQ_RECORD_STEAL KVM_ARCH_REQ(3)
>>
>> DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
>>
>> @@ -77,6 +78,12 @@ struct kvm_arch {
>>
>> /* Mandated version of PSCI */
>> u32 psci_version;
>> +
>> + struct kvm_arch_pvtime {
>> + struct gfn_to_hva_cache st_ghc;
>> + gpa_t st_base;
>> + u64 st_size;
>> + } pvtime;
>
> It'd be good if we could avoid having this in the 32bit vcpu structure,
> given that it serves no real purpose (other than being able to compile
> things).
Good point - I think I can fix that with a couple more static inline
functions... It's a little tricky due to header file include order, but
I think I can make it work.
[...]
>> +int kvm_update_stolen_time(struct kvm_vcpu *vcpu, bool init)
>> +{
>> + struct kvm *kvm = vcpu->kvm;
>> + struct kvm_arch_pvtime *pvtime = &kvm->arch.pvtime;
>> + u64 steal;
>> + u64 steal_le;
>> + u64 offset;
>> + int idx;
>> + const int stride = sizeof(struct pvclock_vcpu_stolen_time);
>> +
>> + if (pvtime->st_base == GPA_INVALID)
>> + return -ENOTSUPP;
>> +
>> + /* Let's do the local bookkeeping */
>> + steal = vcpu->arch.steal.steal;
>> + steal += current->sched_info.run_delay - vcpu->arch.steal.last_steal;
>> + vcpu->arch.steal.last_steal = current->sched_info.run_delay;
>> + vcpu->arch.steal.steal = steal;
>> +
>> + offset = stride * kvm_vcpu_get_idx(vcpu);
>> +
>> + if (unlikely(offset + stride > pvtime->st_size))
>> + return -EINVAL;
>> +
>> + steal_le = cpu_to_le64(steal);
>> + pagefault_disable();
>
> What's the reason for doing a pagefault_disable()? What I'd expect is
> for the userspace page to be faulted in and written to, and doing a
> pagefault_disable() seems to be going against this idea.
Umm... this is me screwing up the locking...
The current code is very confused about which locks should/can be held
when kvm_update_stolen_time() is called. vcpu_req_record_steal()
explicitly takes the kvm->srcu read lock - which is then taken again
here. But kvm_hypercall_stolen_time doesn't hold any lock. And obviously
at some point in time I expected this to be called in atomic context...
In general the page is likely to be faulted in (as a guest which is
using stolen time is surely looking at the numbers there). But there's
no need for the pagefault_disable(). It also shouldn't be the callers
responsibility to hold kvm->srcu.
Steve
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 14/15] drm/mxsfb: Add support for horizontal stride
From: Robert Chiras @ 2019-08-21 10:15 UTC (permalink / raw)
To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Fabio Estevam
Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel
In-Reply-To: <1566382555-12102-1-git-send-email-robert.chiras@nxp.com>
Besides the eLCDIF block, there is another IP block, used in the past
for EPDC panels. Since the iMX.8mq doesn't have an EPDC connector, this
block is not documented, but we can use it to do additional operations
on the frame buffer.
In this case, we can use the pigeon registers from this IP block in
order to do horizontal crop on the frame buffer processed by the eLCDIF
block.
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 79 ++++++++++++++++++++++++++++++++++++--
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 1 +
drivers/gpu/drm/mxsfb/mxsfb_regs.h | 16 ++++++++
3 files changed, 92 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index a12f53d..317575e 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -15,6 +15,7 @@
#include <video/videomode.h>
+#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_crtc.h>
#include <drm/drm_fb_cma_helper.h>
@@ -435,13 +436,66 @@ void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
clk_disable_unprepare(mxsfb->clk_axi);
}
+void mxsfb_set_fb_hcrop(struct mxsfb_drm_private *mxsfb, u32 src_w, u32 fb_w)
+{
+ u32 mask_cnt, htotal, hcount;
+ u32 vdctrl2, vdctrl3, vdctrl4, transfer_count;
+ u32 pigeon_12_0, pigeon_12_1, pigeon_12_2;
+
+ if (src_w == fb_w) {
+ writel(0x0, mxsfb->base + HW_EPDC_PIGEON_12_0);
+ writel(0x0, mxsfb->base + HW_EPDC_PIGEON_12_1);
+
+ return;
+ }
+
+ transfer_count = readl(mxsfb->base + LCDC_V4_TRANSFER_COUNT);
+ hcount = TRANSFER_COUNT_GET_HCOUNT(transfer_count);
+
+ transfer_count &= ~TRANSFER_COUNT_SET_HCOUNT(0xffff);
+ transfer_count |= TRANSFER_COUNT_SET_HCOUNT(fb_w);
+ writel(transfer_count, mxsfb->base + LCDC_V4_TRANSFER_COUNT);
+
+ vdctrl2 = readl(mxsfb->base + LCDC_VDCTRL2);
+ htotal = VDCTRL2_GET_HSYNC_PERIOD(vdctrl2);
+ htotal += fb_w - hcount;
+ vdctrl2 &= ~VDCTRL2_SET_HSYNC_PERIOD(0x3ffff);
+ vdctrl2 |= VDCTRL2_SET_HSYNC_PERIOD(htotal);
+ writel(vdctrl2, mxsfb->base + LCDC_VDCTRL2);
+
+ vdctrl4 = readl(mxsfb->base + LCDC_VDCTRL4);
+ vdctrl4 &= ~SET_DOTCLK_H_VALID_DATA_CNT(0x3ffff);
+ vdctrl4 |= SET_DOTCLK_H_VALID_DATA_CNT(fb_w);
+ writel(vdctrl4, mxsfb->base + LCDC_VDCTRL4);
+
+ /* configure related pigeon registers */
+ vdctrl3 = readl(mxsfb->base + LCDC_VDCTRL3);
+ mask_cnt = GET_HOR_WAIT_CNT(vdctrl3) - 5;
+
+ pigeon_12_0 = PIGEON_12_0_SET_STATE_MASK(0x24) |
+ PIGEON_12_0_SET_MASK_CNT(mask_cnt) |
+ PIGEON_12_0_SET_MASK_CNT_SEL(0x6) |
+ PIGEON_12_0_POL_ACTIVE_LOW |
+ PIGEON_12_0_EN;
+ writel(pigeon_12_0, mxsfb->base + HW_EPDC_PIGEON_12_0);
+
+ pigeon_12_1 = PIGEON_12_1_SET_CLR_CNT(src_w) |
+ PIGEON_12_1_SET_SET_CNT(0x0);
+ writel(pigeon_12_1, mxsfb->base + HW_EPDC_PIGEON_12_1);
+
+ pigeon_12_2 = 0x0;
+ writel(pigeon_12_2, mxsfb->base + HW_EPDC_PIGEON_12_2);
+}
+
void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
struct drm_plane_state *state)
{
struct drm_simple_display_pipe *pipe = &mxsfb->pipe;
struct drm_crtc *crtc = &pipe->crtc;
+ struct drm_plane_state *new_state = pipe->plane.state;
+ struct drm_framebuffer *fb = pipe->plane.state->fb;
struct drm_pending_vblank_event *event;
- dma_addr_t paddr;
+ u32 fb_addr, src_off, src_w, stride, cpp = 0;
spin_lock_irq(&crtc->dev->event_lock);
event = crtc->state->event;
@@ -456,10 +510,27 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
}
spin_unlock_irq(&crtc->dev->event_lock);
- paddr = mxsfb_get_fb_paddr(mxsfb);
- if (paddr) {
+ if (!fb)
+ return;
+
+ fb_addr = mxsfb_get_fb_paddr(mxsfb);
+ if (mxsfb->devdata->ipversion >= 4) {
+ cpp = fb->format->cpp[0];
+ src_off = (new_state->src_y >> 16) * fb->pitches[0] +
+ (new_state->src_x >> 16) * cpp;
+ fb_addr += fb->offsets[0] + src_off;
+ }
+
+ if (fb_addr) {
clk_prepare_enable(mxsfb->clk_axi);
- writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
+ writel(fb_addr, mxsfb->base + mxsfb->devdata->next_buf);
clk_disable_unprepare(mxsfb->clk_axi);
}
+
+ if (mxsfb->devdata->ipversion >= 4 &&
+ unlikely(drm_atomic_crtc_needs_modeset(new_state->crtc->state))) {
+ stride = DIV_ROUND_UP(fb->pitches[0], cpp);
+ src_w = new_state->src_w >> 16;
+ mxsfb_set_fb_hcrop(mxsfb, src_w, stride);
+ }
}
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index dd5809b..c4ce706 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -136,6 +136,7 @@ static int mxsfb_atomic_helper_check(struct drm_device *dev,
if (old_bpp != new_bpp)
new_state->mode_changed = true;
}
+
return ret;
}
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index 0f63ba1..df3279b 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -145,6 +145,22 @@
#define DEBUG0_HSYNC BIT(26)
#define DEBUG0_VSYNC BIT(25)
+/* pigeon registers for crop */
+#define HW_EPDC_PIGEON_12_0 0xb00
+#define HW_EPDC_PIGEON_12_1 0xb10
+#define HW_EPDC_PIGEON_12_2 0xb20
+
+#define PIGEON_12_0_SET_STATE_MASK(x) REG_PUT((x), 31, 24)
+#define PIGEON_12_0_SET_MASK_CNT(x) REG_PUT((x), 23, 12)
+#define PIGEON_12_0_SET_MASK_CNT_SEL(x) REG_PUT((x), 11, 8)
+#define PIGEON_12_0_SET_OFFSET(x) REG_PUT((x), 7, 4)
+#define PIGEON_12_0_SET_INC_SEL(x) REG_PUT((x), 3, 2)
+#define PIGEON_12_0_POL_ACTIVE_LOW BIT(1)
+#define PIGEON_12_0_EN BIT(0)
+
+#define PIGEON_12_1_SET_CLR_CNT(x) REG_PUT((x), 31, 16)
+#define PIGEON_12_1_SET_SET_CNT(x) REG_PUT((x), 15, 0)
+
#define MXSFB_MIN_XRES 120
#define MXSFB_MIN_YRES 120
#define MXSFB_MAX_XRES 0xffff
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 15/15] drm/mxsfb: Add support for live pixel format change
From: Robert Chiras @ 2019-08-21 10:15 UTC (permalink / raw)
To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Fabio Estevam
Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel
In-Reply-To: <1566382555-12102-1-git-send-email-robert.chiras@nxp.com>
This IP requires full stop and re-start when changing display timings,
but we can change the pixel format while running.
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index 317575e..5607fc0 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -494,6 +494,7 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
struct drm_crtc *crtc = &pipe->crtc;
struct drm_plane_state *new_state = pipe->plane.state;
struct drm_framebuffer *fb = pipe->plane.state->fb;
+ struct drm_framebuffer *old_fb = state->fb;
struct drm_pending_vblank_event *event;
u32 fb_addr, src_off, src_w, stride, cpp = 0;
@@ -510,7 +511,7 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
}
spin_unlock_irq(&crtc->dev->event_lock);
- if (!fb)
+ if (!fb || !old_fb)
return;
fb_addr = mxsfb_get_fb_paddr(mxsfb);
@@ -533,4 +534,17 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
src_w = new_state->src_w >> 16;
mxsfb_set_fb_hcrop(mxsfb, src_w, stride);
}
+
+ if (old_fb->format->format != fb->format->format) {
+ struct drm_format_name_buf old_fmt_buf;
+ struct drm_format_name_buf new_fmt_buf;
+
+ DRM_DEV_DEBUG_DRIVER(crtc->dev->dev,
+ "Switching pixel format: %s -> %s\n",
+ drm_get_format_name(old_fb->format->format,
+ &old_fmt_buf),
+ drm_get_format_name(fb->format->format,
+ &new_fmt_buf));
+ mxsfb_set_pixel_fmt(mxsfb, true);
+ }
}
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 13/15] drm/mxsfb: Clear OUTSTANDING_REQS bits
From: Robert Chiras @ 2019-08-21 10:15 UTC (permalink / raw)
To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Fabio Estevam
Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel
In-Reply-To: <1566382555-12102-1-git-send-email-robert.chiras@nxp.com>
Bit 21 can alter the CTRL2_OUTSTANDING_REQS value right after the eLCDIF
is enabled, since it comes up with default value of 1 (this behaviour
has been seen on some imx8 platforms).
In order to fix this, clear CTRL2_OUTSTANDING_REQS bits before setting
its value.
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index e727f5e..a12f53d 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -225,6 +225,13 @@ static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb)
clk_prepare_enable(mxsfb->clk);
if (mxsfb->devdata->ipversion >= 4) {
+ /*
+ * On some platforms, bit 21 is defaulted to 1, which may alter
+ * the below setting. So, to make sure we have the right setting
+ * clear all the bits for CTRL2_OUTSTANDING_REQS.
+ */
+ writel(CTRL2_OUTSTANDING_REQS(0x7),
+ mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
writel(CTRL2_OUTSTANDING_REQS(REQ_16),
mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
/* Assert LCD Reset bit */
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 12/15] drm/mxsfb: Improve the axi clock usage
From: Robert Chiras @ 2019-08-21 10:15 UTC (permalink / raw)
To: Guido Günther, Marek Vasut, Stefan Agner, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Fabio Estevam
Cc: devicetree, linux-kernel, dri-devel, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel
In-Reply-To: <1566382555-12102-1-git-send-email-robert.chiras@nxp.com>
Currently, the enable of the axi clock return status is ignored, causing
issues when the enable fails then we try to disable it. Therefore, it is
better to check the return status and disable it only when enable
succeeded.
Also, remove the helper functions around clk_axi, since we can directly
use the clk API function for enable/disable the clock. Those functions
are already checking for NULL clk and returning 0 if that's the case.
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Acked-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 8 ++++----
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 32 +++++++++++++-------------------
drivers/gpu/drm/mxsfb/mxsfb_drv.h | 3 ---
3 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index a4ba368..e727f5e 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -408,7 +408,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
{
dma_addr_t paddr;
- mxsfb_enable_axi_clk(mxsfb);
+ clk_prepare_enable(mxsfb->clk_axi);
writel(0, mxsfb->base + LCDC_CTRL);
mxsfb_crtc_mode_set_nofb(mxsfb);
@@ -425,7 +425,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
{
mxsfb_disable_controller(mxsfb);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
}
void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
@@ -451,8 +451,8 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
paddr = mxsfb_get_fb_paddr(mxsfb);
if (paddr) {
- mxsfb_enable_axi_clk(mxsfb);
+ clk_prepare_enable(mxsfb->clk_axi);
writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
}
}
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index deb5e2b..dd5809b 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -97,18 +97,6 @@ drm_pipe_to_mxsfb_drm_private(struct drm_simple_display_pipe *pipe)
return container_of(pipe, struct mxsfb_drm_private, pipe);
}
-void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
-{
- if (mxsfb->clk_axi)
- clk_prepare_enable(mxsfb->clk_axi);
-}
-
-void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
-{
- if (mxsfb->clk_axi)
- clk_disable_unprepare(mxsfb->clk_axi);
-}
-
/**
* mxsfb_atomic_helper_check - validate state object
* @dev: DRM device
@@ -272,25 +260,31 @@ static void mxsfb_pipe_update(struct drm_simple_display_pipe *pipe,
static int mxsfb_pipe_enable_vblank(struct drm_simple_display_pipe *pipe)
{
struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
+ int ret = 0;
+
+ ret = clk_prepare_enable(mxsfb->clk_axi);
+ if (ret)
+ return ret;
/* Clear and enable VBLANK IRQ */
- mxsfb_enable_axi_clk(mxsfb);
writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_SET);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
- return 0;
+ return ret;
}
static void mxsfb_pipe_disable_vblank(struct drm_simple_display_pipe *pipe)
{
struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
+ if (clk_prepare_enable(mxsfb->clk_axi))
+ return;
+
/* Disable and clear VBLANK IRQ */
- mxsfb_enable_axi_clk(mxsfb);
writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_CLR);
writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
}
static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
@@ -451,7 +445,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
struct mxsfb_drm_private *mxsfb = drm->dev_private;
u32 reg;
- mxsfb_enable_axi_clk(mxsfb);
+ clk_prepare_enable(mxsfb->clk_axi);
reg = readl(mxsfb->base + LCDC_CTRL1);
@@ -460,7 +454,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
- mxsfb_disable_axi_clk(mxsfb);
+ clk_disable_unprepare(mxsfb->clk_axi);
return IRQ_HANDLED;
}
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index a178173..54c0644 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -39,9 +39,6 @@ struct mxsfb_drm_private {
int mxsfb_setup_crtc(struct drm_device *dev);
int mxsfb_create_output(struct drm_device *dev);
-void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb);
-void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb);
-
void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb);
void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb);
void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox