* [PATCH V2 4/6] ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS
From: Will Deacon @ 2014-01-24 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390581656-16372-5-git-send-email-adrienverge@gmail.com>
On Fri, Jan 24, 2014 at 04:40:54PM +0000, Adrien Verg? wrote:
> When using namespaces, different processes can have the same PID.
> It makes no sense to store a PID value in the Context ID register
> to track a specific process, when others share the same value.
>
> Consequently, PID_IN_CONTEXTIDR (which is used for tracing and
> debugging processes) should not be compatible with PID_NS.
>
> Signed-off-by: Adrien Verg? <adrienverge@gmail.com>
> ---
> arch/arm/Kconfig.debug | 2 +-
> arch/arm64/Kconfig.debug | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 5765abf..ed46748 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -1143,7 +1143,7 @@ config ARM_KPROBES_TEST
>
> config PID_IN_CONTEXTIDR
> bool "Write the current PID to the CONTEXTIDR register"
> - depends on CPU_COPY_V6
> + depends on CPU_COPY_V6 && !PID_NS
> help
> Enabling this option causes the kernel to write the current PID to
> the PROCID field of the CONTEXTIDR register, at the expense of some
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index 835c559..06b2633b 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -17,6 +17,7 @@ config EARLY_PRINTK
>
> config PID_IN_CONTEXTIDR
> bool "Write the current PID to the CONTEXTIDR register"
> + depends on !PID_NS
> help
> Enabling this option causes the kernel to write the current PID to
> the CONTEXTIDR register, at the expense of some additional
Are you sure about this? The value we write is actually task_pid_nr, which I
believe to be globally unique.
Will
^ permalink raw reply
* [Linaro-acpi] [PATCH 04/20] ARM64 / ACPI: Introduce arm_core.c and its related head file
From: Tomasz Nowicki @ 2014-01-24 16:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124125332.GA23274@e102568-lin.cambridge.arm.com>
Hi Lorenzo,
W dniu 24.01.2014 13:53, Lorenzo Pieralisi pisze:
> Hi Hanjun,
>
> On Fri, Jan 24, 2014 at 09:09:40AM +0000, Hanjun Guo wrote:
>> On 2014?01?23? 23:56, Tomasz Nowicki wrote:
>>> Hi Lorenzo,
>>>
>>> W dniu 22.01.2014 12:54, Lorenzo Pieralisi pisze:
>>>> On Fri, Jan 17, 2014 at 12:24:58PM +0000, Hanjun Guo wrote:
>>>>
>>>> [...]
>>>>
>>>>> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
>>>>> index bd9bbd0..2210353 100644
>>>>> --- a/arch/arm64/kernel/setup.c
>>>>> +++ b/arch/arm64/kernel/setup.c
>>>>> @@ -41,6 +41,7 @@
>>>>> #include <linux/memblock.h>
>>>>> #include <linux/of_fdt.h>
>>>>> #include <linux/of_platform.h>
>>>>> +#include <linux/acpi.h>
>>>>>
>>>>> #include <asm/cputype.h>
>>>>> #include <asm/elf.h>
>>>>> @@ -225,6 +226,11 @@ void __init setup_arch(char **cmdline_p)
>>>>>
>>>>> arm64_memblock_init();
>>>>>
>>>>> + /* Parse the ACPI tables for possible boot-time configuration */
>>>>> + acpi_boot_table_init();
>>>>> + early_acpi_boot_init();
>>>>> + acpi_boot_init();
>>>>> +
>>>>> paging_init();
>>>>
>>>> Can I ask you please why we need to parse ACPI tables before
>>>> paging_init() ?
>>> This is for future usage and because of couple of reasons. Mainly SRAT
>>> table parsing should be done (before paging_init()) for proper NUMA
>>> initialization and then paging_init().
>
> Thank you for the explanation. I still have some questions:
>
> 1) What are the other reasons ?
If we agreed that we need SRAT parsing before paging_init() the second
reason is all implications related to that. I mean if we want to make
ACPI tables parseable before paging_init(), we need early_ioremap
mechanism and __acpi_map_table() fixes. In that case, IMHO, it is better
to please it in the right place now. early_ioremap is object of UEFI
support patch set.
> 2) NUMA is not supported at the moment and I reckon SRAT needs updating
> since the only way to associate a CPU to a memory node is through
> a local APIC id that is non-existent on ARM and at least deserves a
> new entry.
Right, that requires further work on SRAT.
>
> I am still not sure that providing a hook for parsing the ACPI tables before
> paging_init() is the main focus at the moment, it is probably best, as we've
> mentioned manifold times, to make sure that the infrastructure to detect
> ACPI vs DT at run-time is in place in the kernel and allows us to boot
> either with ACPI or DT, in a mutual exclusive way (same binary kernel
> supporting both, runtime detection/decision on what data to use, ACPI tables
> vs DT nodes, detection made once for all and NOT on a per property basis).
>
> I will have a look at SRAT and how it is used on x86, and get back to you on
> this.
>
> [...]
>
>>>>> + * acpi_boot_table_init() and acpi_boot_init()
>>>>> + * called from setup_arch(), always.
>>>>> + * 1. checksums all tables
>>>>> + * 2. enumerates lapics
>>>>> + * 3. enumerates io-apics
>>>>> + *
>>>>> + * acpi_table_init() is separated to allow reading SRAT without
>>>>> + * other side effects.
>>>>> + */
>>>>> +void __init acpi_boot_table_init(void)
>>>>> +{
>>>>> + /*
>>>>> + * If acpi_disabled, bail out
>>>>> + */
>>>>> + if (acpi_disabled)
>>>>> + return;
>>>>> +
>>>>> + /*
>>>>> + * Initialize the ACPI boot-time table parser.
>>>>> + */
>>>>> + if (acpi_table_init()) {
>>>>> + disable_acpi();
>>>>> + return;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +int __init early_acpi_boot_init(void)
>>>>> +{
>>>>> + /*
>>>>> + * If acpi_disabled, bail out
>>>>> + */
>>>>> + if (acpi_disabled)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + /*
>>>>> + * Process the Multiple APIC Description Table (MADT), if present
>>>>> + */
>>>>> + early_acpi_process_madt();
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int __init acpi_boot_init(void)
>>>>> +{
>>>>> + /*
>>>>> + * If acpi_disabled, bail out
>>>>> + */
>>>>> + if (acpi_disabled)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt);
>>>>> +
>>>>> + /*
>>>>> + * Process the Multiple APIC Description Table (MADT), if present
>>>>> + */
>>>>> + acpi_process_madt();
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>
>>>> Well, apart from having three init calls, one returning void and two
>>>> returning proper values, do not understand why, and do not understand
>>>> why we need three calls in the first place...why should we process MADT
>>>> twice in two separate calls ? What is supposed to change in between that
>>>> prevents you from merging the two together ?
>>
>> Thanks for pointing this out. I can merge acpi_boot_table_init() and
>> early_acpi_boot_init() together, but can not merge early_acpi_boot_init()
>> and acpi_boot_init() together.
>>
>> early_acpi_boot_init() and acpi_boot_init() was separated intentionally for
>> memory hotplug reasons. memory allocated in this stage can not be migrated
>> and cause memory hot-remove failed, in order to keep memory allocated
>> at base node (general NUMA node 0 in the system) at boot stage, we should
>> parse SRAT first before CPU is enumerated, does this make sense to you?
>
> Are you parsing the SRAT in this series to get memory info or memory is
> still initialized by DT even when system is supposed to be booted with ACPI
> (ie there is a valid ACPI root table ?)
>
> I have a hunch the latter is what's happening (and that's wrong, because
> memory information when kernel is booted through ACPI must be retrieved
> from UEFI - at least that's what has been defined), so I still see an early
> hook to initialize ACPI tables before paging_init() that has no use as the
> current patchset stands, please correct me if I am wrong.
Yes, currently memory is initialized by DT but getting memory map from
UEFI is the matter of time (pending upstream process). Also, SRAT table
is not parsing in this series but it will, and I think it plays the main
role to put it before paging_init.
Regards,
Tomasz
^ permalink raw reply
* [PATCH/RFC] dmaengine: omap-dma: split header file
From: Balaji T K @ 2014-01-24 16:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140119100235.GN27282@n2100.arm.linux.org.uk>
To Resolve build failure seen with sh-allmodconfig:
include/linux/omap-dma.h:171:8: error: expected identifier before numeric constant
make[4]: *** [drivers/mmc/host/omap_hsmmc.o] Error 1
due to CCR redefinition, move dmaengine consumer specific function to omap-dmaengine.h
Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Balaji T K <balajitk@ti.com>
---
drivers/mmc/host/omap_hsmmc.c | 2 +-
include/linux/omap-dma.h | 19 +------------------
include/linux/omap-dmaengine.h | 21 +++++++++++++++++++++
3 files changed, 23 insertions(+), 19 deletions(-)
create mode 100644 include/linux/omap-dmaengine.h
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index dbd32ad..2f57e36 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -31,7 +31,7 @@
#include <linux/of.h>
#include <linux/of_gpio.h>
#include <linux/of_device.h>
-#include <linux/omap-dma.h>
+#include <linux/omap-dmaengine.h>
#include <linux/mmc/host.h>
#include <linux/mmc/core.h>
#include <linux/mmc/mmc.h>
diff --git a/include/linux/omap-dma.h b/include/linux/omap-dma.h
index 7af25a9..6aa97e5 100644
--- a/include/linux/omap-dma.h
+++ b/include/linux/omap-dma.h
@@ -1,23 +1,6 @@
-/*
- * OMAP DMA Engine support
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
#ifndef __LINUX_OMAP_DMA_H
#define __LINUX_OMAP_DMA_H
-
-struct dma_chan;
-
-#if defined(CONFIG_DMA_OMAP) || defined(CONFIG_DMA_OMAP_MODULE)
-bool omap_dma_filter_fn(struct dma_chan *, void *);
-#else
-static inline bool omap_dma_filter_fn(struct dma_chan *c, void *d)
-{
- return false;
-}
-#endif
+#include <linux/omap-dmaengine.h>
/*
* Legacy OMAP DMA handling defines and functions
diff --git a/include/linux/omap-dmaengine.h b/include/linux/omap-dmaengine.h
new file mode 100644
index 0000000..2b0b6aa
--- /dev/null
+++ b/include/linux/omap-dmaengine.h
@@ -0,0 +1,21 @@
+/*
+ * OMAP DMA Engine support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef __LINUX_OMAP_DMAENGINE_H
+#define __LINUX_OMAP_DMAENGINE_H
+
+struct dma_chan;
+
+#if defined(CONFIG_DMA_OMAP) || defined(CONFIG_DMA_OMAP_MODULE)
+bool omap_dma_filter_fn(struct dma_chan *, void *);
+#else
+static inline bool omap_dma_filter_fn(struct dma_chan *c, void *d)
+{
+ return false;
+}
+#endif
+#endif /* __LINUX_OMAP_DMAENGINE_H */
--
1.7.5.4
^ permalink raw reply related
* [PATCH/RFC] dmaengine: omap-dma: split header file
From: Balaji T K @ 2014-01-24 16:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390582316-4765-1-git-send-email-balajitk@ti.com>
On Friday 24 January 2014 10:21 PM, Balaji T K wrote:
> To Resolve build failure seen with sh-allmodconfig:
> include/linux/omap-dma.h:171:8: error: expected identifier before numeric constant
> make[4]: *** [drivers/mmc/host/omap_hsmmc.o] Error 1
> due to CCR redefinition, move dmaengine consumer specific function to omap-dmaengine.h
>
> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
s/Tested-by/Reported-by
> Signed-off-by: Balaji T K <balajitk@ti.com>
> ---
> drivers/mmc/host/omap_hsmmc.c | 2 +-
> include/linux/omap-dma.h | 19 +------------------
> include/linux/omap-dmaengine.h | 21 +++++++++++++++++++++
> 3 files changed, 23 insertions(+), 19 deletions(-)
> create mode 100644 include/linux/omap-dmaengine.h
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index dbd32ad..2f57e36 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -31,7 +31,7 @@
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> #include <linux/of_device.h>
> -#include <linux/omap-dma.h>
> +#include <linux/omap-dmaengine.h>
> #include <linux/mmc/host.h>
> #include <linux/mmc/core.h>
> #include <linux/mmc/mmc.h>
> diff --git a/include/linux/omap-dma.h b/include/linux/omap-dma.h
> index 7af25a9..6aa97e5 100644
> --- a/include/linux/omap-dma.h
> +++ b/include/linux/omap-dma.h
> @@ -1,23 +1,6 @@
> -/*
> - * OMAP DMA Engine support
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - */
> #ifndef __LINUX_OMAP_DMA_H
> #define __LINUX_OMAP_DMA_H
> -
> -struct dma_chan;
> -
> -#if defined(CONFIG_DMA_OMAP) || defined(CONFIG_DMA_OMAP_MODULE)
> -bool omap_dma_filter_fn(struct dma_chan *, void *);
> -#else
> -static inline bool omap_dma_filter_fn(struct dma_chan *c, void *d)
> -{
> - return false;
> -}
> -#endif
> +#include <linux/omap-dmaengine.h>
>
> /*
> * Legacy OMAP DMA handling defines and functions
> diff --git a/include/linux/omap-dmaengine.h b/include/linux/omap-dmaengine.h
> new file mode 100644
> index 0000000..2b0b6aa
> --- /dev/null
> +++ b/include/linux/omap-dmaengine.h
> @@ -0,0 +1,21 @@
> +/*
> + * OMAP DMA Engine support
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __LINUX_OMAP_DMAENGINE_H
> +#define __LINUX_OMAP_DMAENGINE_H
> +
> +struct dma_chan;
> +
> +#if defined(CONFIG_DMA_OMAP) || defined(CONFIG_DMA_OMAP_MODULE)
> +bool omap_dma_filter_fn(struct dma_chan *, void *);
> +#else
> +static inline bool omap_dma_filter_fn(struct dma_chan *c, void *d)
> +{
> + return false;
> +}
> +#endif
> +#endif /* __LINUX_OMAP_DMAENGINE_H */
>
^ permalink raw reply
* [alsa-devel] [PATCH RFC v2 REPOST 3/8] ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus
From: Jean-Francois Moine @ 2014-01-24 16:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E26325.4040203@ti.com>
On Fri, 24 Jan 2014 14:57:09 +0200
Jyri Sarha <jsarha@ti.com> wrote:
> Could you give me a link to a git repo with your tda998x codec code so I
> could prepare to use that too?
I have no git repo. Instead, here is the source.
It goes into sound/soc/codecs and is selected with CONFIG_OF and
CONFIG_DRM_I2C_NXP_TDA998X.
It needs 2 exported functions of the tda998x driver: the first one to
select the audio port (tda998x_audio_update) and the other one to get
the ELD (tda998x_audio_get_eld).
For these functions to receive the correct i2c_client, the codec
searches the tda998x driver in the DT by its compatible "nxp,tda998x".
If the tda998x is loaded by drm_i2c_encoder_init(), it should not be
declared in the DT, so, this raises a problem. I don't know what must
be done in this case.
The codec is used with the simple card as the sound card. For you, the
DT could be:
hdmi_codec: hdmi-codec {
compatible = "nxp,tda998x-codec";
#sound-dai-cells = <1>;
audio-ports = <0x03>, <0x04>; /* i2s - s/pdif */
};
sound {
compatible = "simple-audio-card";
simple-audio-card,cpu {
sound-dai = <&audio1 1>;
format = "i2s";
};
simple-audio-card,codec {
sound-dai = <&hdmi_codec 0>; /* i2s */
};
};
('audio1' is the audio controller)
-------------------8<---------------- source sound/soc/codecs/tda998x.c
/*
* ALSA SoC TDA998X driver
*
* This driver is used by the NXP TDA998x HDMI transmitter.
*
* Copyright (C) 2014 Jean-Francois Moine
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
#include <linux/module.h>
#include <sound/soc.h>
#include <sound/pcm.h>
#include <linux/of.h>
#include <linux/i2c.h>
#include <drm/drm_encoder_slave.h>
#include <drm/i2c/tda998x.h>
#define TDA998X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
SNDRV_PCM_FMTBIT_S20_3LE | \
SNDRV_PCM_FMTBIT_S24_LE | \
SNDRV_PCM_FMTBIT_S32_LE)
struct tda_priv {
struct i2c_client *i2c_client;
struct snd_soc_codec *codec;
u32 ports[2];
int dai_id;
u8 *eld;
};
static void tda_get_encoder(struct tda_priv *priv)
{
struct snd_soc_codec *codec = priv->codec;
struct device_node *np;
struct i2c_client *i2c_client;
static const struct of_device_id tda_dt[] = {
{ .compatible = "nxp,tda998x" },
{ },
};
/* search the tda998x device (only one!) */
np = of_find_matching_node_and_match(NULL, tda_dt, NULL);
if (!np || !of_device_is_available(np)) {
dev_err(codec->dev, "No tda998x in DT\n");
return;
}
i2c_client = of_find_i2c_device_by_node(np);
of_node_put(np);
if (!i2c_client) {
dev_err(codec->dev, "no tda998x i2c client\n");
return;
}
if (!i2c_get_clientdata(i2c_client)) {
dev_err(codec->dev, "tda998x not initialized\n");
return;
}
priv->i2c_client = i2c_client;
}
static int tda_start_stop(struct tda_priv *priv,
int start)
{
int format;
u32 port;
if (!priv->i2c_client) {
tda_get_encoder(priv);
if (!priv->i2c_client)
return -EINVAL;
}
/* give the audio input type and ports to the HDMI encoder */
format = start ? priv->dai_id : 0;
switch (format) {
case AFMT_I2S:
port = priv->ports[0];
break;
default:
case AFMT_SPDIF:
port = priv->ports[1];
break;
}
tda998x_audio_update(priv->i2c_client, format, port);
return 0;
}
static int tda_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec);
u8 *eld = NULL;
static unsigned rates_mask[] = {
SNDRV_PCM_RATE_32000,
SNDRV_PCM_RATE_44100,
SNDRV_PCM_RATE_48000,
SNDRV_PCM_RATE_88200,
SNDRV_PCM_RATE_96000,
SNDRV_PCM_RATE_176400,
SNDRV_PCM_RATE_192000,
};
/* memorize the used DAI */
priv->dai_id = dai->id;
/* get the ELD from the tda998x driver */
if (!priv->i2c_client)
tda_get_encoder(priv);
if (priv->i2c_client)
eld = tda998x_audio_get_eld(priv->i2c_client);
/* limit the hw params from the ELD (EDID) */
if (eld) {
struct snd_soc_dai_driver *dai_drv = dai->driver;
struct snd_soc_pcm_stream *stream = &dai_drv->playback;
u8 *sad;
int sad_count;
unsigned eld_ver, mnl, rates, rate_mask, i;
unsigned max_channels;
eld_ver = eld[0] >> 3;
if (eld_ver != 2 && eld_ver != 31)
return 0;
mnl = eld[4] & 0x1f;
if (mnl > 16)
return 0;
sad_count = eld[5] >> 4;
sad = eld + 20 + mnl;
/* Start from the basic audio settings */
max_channels = 2;
rates = 0;
while (sad_count--) {
switch (sad[0] & 0x78) {
case 0x08: /* PCM */
max_channels = max(max_channels, (sad[0] & 7) + 1u);
rates |= sad[1];
break;
}
sad += 3;
}
for (rate_mask = i = 0; i < ARRAY_SIZE(rates_mask); i++)
if (rates & 1 << i)
rate_mask |= rates_mask[i];
/* change the snd_soc_pcm_stream values of the driver */
stream->rates = rate_mask;
stream->channels_max = max_channels;
}
/* start the TDA998x audio */
return tda_start_stop(priv, 1);
}
static void tda_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec);
priv->dai_id = 0;
tda_start_stop(priv, 0);
}
static const struct snd_soc_dai_ops tda_ops = {
.startup = tda_startup,
.shutdown = tda_shutdown,
};
static const struct snd_soc_dai_driver tda998x_dai[] = {
{
.name = "i2s-hifi",
.id = AFMT_I2S,
.playback = {
.stream_name = "HDMI I2S Playback",
.channels_min = 1,
.channels_max = 8,
.rates = SNDRV_PCM_RATE_CONTINUOUS,
.rate_min = 5512,
.rate_max = 192000,
.formats = TDA998X_FORMATS,
},
.ops = &tda_ops,
},
{
.name = "spdif-hifi",
.id = AFMT_SPDIF,
.playback = {
.stream_name = "HDMI SPDIF Playback",
.channels_min = 1,
.channels_max = 2,
.rates = SNDRV_PCM_RATE_CONTINUOUS,
.rate_min = 22050,
.rate_max = 192000,
.formats = TDA998X_FORMATS,
},
.ops = &tda_ops,
},
};
static const struct snd_soc_dapm_widget tda_widgets[] = {
SND_SOC_DAPM_OUTPUT("hdmi-out"),
};
static const struct snd_soc_dapm_route tda_routes[] = {
{ "hdmi-out", NULL, "HDMI I2S Playback" },
{ "hdmi-out", NULL, "HDMI SPDIF Playback" },
};
static int tda_probe(struct snd_soc_codec *codec)
{
struct tda_priv *priv;
struct device_node *np;
int i, ret;
priv = devm_kzalloc(codec->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
snd_soc_codec_set_drvdata(codec, priv);
priv->codec = codec;
/* get the audio input ports (I2s and S/PDIF) */
np = codec->dev->of_node;
for (i = 0; i < 2; i++) {
ret = of_property_read_u32_index(np, "audio-ports", i,
&priv->ports[i]);
if (ret) {
if (i == 0)
dev_err(codec->dev,
"bad or missing audio-ports\n");
break;
}
}
return 0;
}
static const struct snd_soc_codec_driver soc_codec_tda998x = {
.probe = tda_probe,
.dapm_widgets = tda_widgets,
.num_dapm_widgets = ARRAY_SIZE(tda_widgets),
.dapm_routes = tda_routes,
.num_dapm_routes = ARRAY_SIZE(tda_routes),
};
static int tda998x_dev_probe(struct platform_device *pdev)
{
struct snd_soc_dai_driver *dai_drv;
/* copy the DAI driver to a writable area */
dai_drv = devm_kzalloc(&pdev->dev, sizeof(tda998x_dai), GFP_KERNEL);
if (!dai_drv)
return -ENOMEM;
memcpy(dai_drv, tda998x_dai, sizeof(tda998x_dai));
return snd_soc_register_codec(&pdev->dev,
&soc_codec_tda998x,
dai_drv, ARRAY_SIZE(tda998x_dai));
}
static int tda998x_dev_remove(struct platform_device *pdev)
{
snd_soc_unregister_codec(&pdev->dev);
return 0;
}
static const struct of_device_id tda998x_codec_ids[] = {
{ .compatible = "nxp,tda998x-codec", },
{ }
};
MODULE_DEVICE_TABLE(of, tda998x_codec_ids);
static struct platform_driver tda998x_driver = {
.probe = tda998x_dev_probe,
.remove = tda998x_dev_remove,
.driver = {
.name = "tda998x-codec",
.owner = THIS_MODULE,
.of_match_table = tda998x_codec_ids,
},
};
module_platform_driver(tda998x_driver);
MODULE_AUTHOR("Jean-Francois Moine");
MODULE_DESCRIPTION("TDA998x codec driver");
MODULE_LICENSE("GPL");
-------------------8<----------------
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [PATCH 2/5] arm64: dts: APM X-Gene SoC Ethernet device tree nodes
From: Mark Salter @ 2014-01-24 17:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1387597376-29303-3-git-send-email-isubramanian@apm.com>
On Fri, 2013-12-20 at 19:42 -0800, Iyappan Subramanian wrote:
> Device tree files for APM X-Gene SoC Ethernet.
>
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
> Signed-off-by: Ravi Patel <rapatel@apm.com>
> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
> ---
> arch/arm64/boot/dts/apm-mustang.dts | 8 ++++++++
> arch/arm64/boot/dts/apm-storm.dtsi | 19 +++++++++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/apm-mustang.dts b/arch/arm64/boot/dts/apm-mustang.dts
> index 1247ca1..ccb2771 100644
> --- a/arch/arm64/boot/dts/apm-mustang.dts
> +++ b/arch/arm64/boot/dts/apm-mustang.dts
> @@ -17,6 +17,10 @@
> model = "APM X-Gene Mustang board";
> compatible = "apm,mustang", "apm,xgene-storm";
>
> + aliases {
> + ethernet0 = &menet;
> + };
> +
> chosen { };
>
> memory {
> @@ -24,3 +28,7 @@
> reg = < 0x1 0x00000000 0x0 0x80000000 >; /* Updated by bootloader */
> };
> };
> +
> +&menet {
> + status = "ok";
> +};
> diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
> index f64f946..9c7b8cd 100644
> --- a/arch/arm64/boot/dts/apm-storm.dtsi
> +++ b/arch/arm64/boot/dts/apm-storm.dtsi
> @@ -204,5 +204,24 @@
> #clock-cells = <1>;
> clocks = <&qmlclk 0>;
> };
> +
> + menet: ethernet at 17020000 {
> + compatible = "apm,xgene-enet";
> + status = "disabled";
> + reg = <0x0 0x17020000 0x0 0x30>,
> + <0x0 0x17020000 0x0 0x10000>,
> + <0x0 0x17020000 0x0 0x20>;
> + slave-name = "RGMII";
> + interrupts = <0x0 0x38 0x4>,
> + <0x0 0x39 0x4>,
> + <0x0 0x3a 0x4>;
> + #clock-cells = <1>;
> + clocks = <ð8clk 0>;
> + local-mac-address = <0x0 0x11 0x3a 0x8a 0x5a 0x78>;
Shouldn't this be a byte string?:
local-mac-address = [ 00 11 3a 8a 5a 78 ];
> + max-frame-size = <0x233a>;
> + devid = <8>;
> + phyid = <3>;
> + phy-mode = "rgmii";
> + };
> };
> };
^ permalink raw reply
* [PATCH v4 5/8] mmc: sdhci-bcm-kona: Add basic use of clocks
From: Tim Kryger @ 2014-01-24 17:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAYSxhoCrveHd5Wkc7Z0qzpGr41qG6-neYSSsuHQuC7AcuJmZQ@mail.gmail.com>
On Tue, Jan 7, 2014 at 11:37 AM, Tim Kryger <tim.kryger@linaro.org> wrote:
> On Sat, Dec 14, 2013 at 12:14 AM, Christian Daudt <bcm@fixthebug.org> wrote:
>> On Thu, Dec 5, 2013 at 11:20 AM, Tim Kryger <tim.kryger@linaro.org> wrote:
>>> Enable the external clock needed by the host controller during the
>>> probe and disable it during the remove.
>>>
>>> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
>>> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
>>> Reviewed-by: Matt Porter <matt.porter@linaro.org>
>>
>> Reviewed-by: Christian Daudt <bcm@fixthebug.org>
>>
>> Chris,
>> Are you ok with me pulling this patch along with the rest of the
>> patchset into the bcm tree?
>>
>> Thanks,
>> csd
>
> Chris,
>
> Can you please provide an answer to csd?
>
> Thanks,
> Tim Kryger
Chris,
I haven't heard back so this patch has been dropped for 3.14 to allow
the remaining patches in the series to move forward. As such, I will
be re-posting this patch for 3.15. It is essential for the
bcm28155_ap board to transition away from fake fixed clocks to the
real clock driver. This is because the bcm281xx clock driver does not
set the CLK_IGNORE_UNUSED flag for the SDHCI peripheral clocks and the
common clock code will shut them off in late init unless the SDHCI
driver has asked for them to be enabled.
Thanks,
Tim
^ permalink raw reply
* [PATCH V2 4/6] ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS
From: Adrien Vergé @ 2014-01-24 17:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124164343.GJ31040@mudshark.cambridge.arm.com>
2014/1/24 Will Deacon <will.deacon@arm.com>:
> Are you sure about this? The value we write is actually task_pid_nr, which I
> believe to be globally unique.
You are right: the task_pid_nr is unique in the system. However when
using namespaces, the so called "PID" is the virtual number that
processes in different namespaces can share.
This PID is the one visible by user-space tasks, in particular
user-space tracers and debuggers. These programs would expect to find
the PID of the traced process in the Context ID reg, while it is not.
I think it is better to remove confusion by making PID_IN_CONTEXTIDR
and PID_NS incompatible.
What do you think?
^ permalink raw reply
* [PATCH V2 4/6] ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS
From: Will Deacon @ 2014-01-24 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANsc=4UgWrFis6hi-0mXZAfery7YGDJ2F+NdZBzq6afwx_i58A@mail.gmail.com>
On Fri, Jan 24, 2014 at 05:16:28PM +0000, Adrien Verg? wrote:
> 2014/1/24 Will Deacon <will.deacon@arm.com>:
> > Are you sure about this? The value we write is actually task_pid_nr, which I
> > believe to be globally unique.
>
> You are right: the task_pid_nr is unique in the system. However when
> using namespaces, the so called "PID" is the virtual number that
> processes in different namespaces can share.
>
> This PID is the one visible by user-space tasks, in particular
> user-space tracers and debuggers. These programs would expect to find
> the PID of the traced process in the Context ID reg, while it is not.
> I think it is better to remove confusion by making PID_IN_CONTEXTIDR
> and PID_NS incompatible.
>
> What do you think?
I think I'd rather have the global ID than disable a potentially useful
feature, especially since this is likely to be consumed by external trace
tools as opposed to user-space tasks.
Will
^ permalink raw reply
* [PATCH v3 15/24] drm/i2c: tda998x: use irq for connection status and EDID read
From: Sebastian Hesselbarth @ 2014-01-24 17:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140122222705.GL15937@n2100.arm.linux.org.uk>
On 01/22/14 23:27, Russell King - ARM Linux wrote:
> On Sun, Jan 19, 2014 at 07:58:43PM +0100, Jean-Francois Moine wrote:
>> This patch adds the optional treatment of the tda998x IRQ.
>>
>> The interrupt function is used to know the display connection status
>> without polling and to speedup reading the EDID.
>>
>> The interrupt number may be defined either in the DT or at encoder set
>> config time for non-DT boards.
>>
>> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
>> ---
[...]
>> @@ -720,6 +787,10 @@ tda998x_encoder_set_config(struct drm_encoder *encoder, void *params)
>> priv->audio_port = p->audio_cfg;
>> priv->audio_format = p->audio_format;
>> }
>> +
>> + priv->irq = p->irq;
>> + if (p->irq)
>> + tda_irq_init(priv);
>
> If we're going to do it this way, this should probably release the IRQ if
> there was one before re-claiming it, just in case this function gets called
> more than once by some driver using it.
>
> The alternative is, as I said before, to use the infrastructure which is
> already there, namely setting the interrupt via struct i2c_client's
> irq member. Yes, that doesn't satisfy Sebastian's comment about using
> a GPIO, but there's no sign of GPIO usage in here at the moment anyway.
> So we might as well use what's already provided.
Russell,
I am fine with using an irq instead of gpio here. I remember you telling
me on a similar patch, that from the gpio you can derive the irq but
not the other way round. Anyway, I also remember reading discussions
about DT gpios vs interrupts, and IIRC the outcome was that passing
interrupts is fine, too.
We usually have both interrupt-controller; and gpio-controller; set on
DT gpio controllers, so let's stick with irq.
And: Thanks for reviewing this again, I am still too busy to keep up
with it.
Sebastian
^ permalink raw reply
* [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Fabio Estevam @ 2014-01-24 17:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140120164859.GF15937@n2100.arm.linux.org.uk>
On Mon, Jan 20, 2014 at 2:48 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> So far so good. Now, what about this external oscillator which has its
> own separate power control. My immediate thought is that this can be
> specified via card_ext_clock - I would simply need to declare a fixed-rate
> clock with either a regulator (power switch) controlled via a gpio (which
> would probably be closer to the hardware) or a gpio as an enable... ah,
> that requires me to write a common clock driver for that bit since this
> is currently not modelled by CCF...
Jiry Sarha posted a gpio controlled clock proposal:
http://www.spinics.net/lists/devicetree/msg16651.html
Regards,
Fabio Estevam
^ permalink raw reply
* [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
From: Grant Likely @ 2014-01-24 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMjrLs9eEPurjUpOfgyexnc__FJkRYmAR84_f7C15HH07Q@mail.gmail.com>
On Mon, 20 Jan 2014 17:03:23 -0800, Olof Johansson <olof@lixom.net> wrote:
> On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely@linaro.org> wrote:
> > On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
> >> >
> >> > Another, more invasive option would be extend the dts syntax and teach
> >> > dtc to handle property appending. Then the soc dts could stay as it is,
> >> > and the board dts could have something like:
> >> >
> >> > /append-property/ interrupts = <&intc1 6 1>;
> >> > /append-property/ interrupt-names = "board-specific-irq";
> >> >
> >> > Both these options solve the issue at the source, are general to other
> >> > properties, and allow more than one level of hierarchy (the proposed
> >> > interrupts-extended-2 only allows one level).
> >>
> >> I've just had a go at implementing the append-property mechanism above
> >> in dtc, and it was far easier than I expected (patch below).
> >>
> >> Does anyone have any issues with the /append-property/ idea?
> >
> > I think that is reasonable.
>
>
> The main problem with this (same for clocks) is if you need to append
> something with a name when the original didn't have any.
>
> Reordering entries might not work for interrupts, since the bindings
> might have requirements on order.
Hmmm, yes. to handle that case would need a higher level construct that
keeps the name and interrupt value together. Maybe something like this?
device {
add-interrupt(27);
add-interrupt(29, 5, controller=&intc1);
add-interrupt(2, 4, name="error");
};
I'm using python-like syntax here where controller and name are optional
arguments.
We could get DTC to generate the appropriate interrupt-parent,
interrupts, interrupts-extended and interrupt-names properties as
needed. Of course the shared context would be lost when the tree is
compiled, but it would be useful for building up parsing data. It would
also work for gpio, clocks and similar bindings. Kind of like an
intelligent macro.
g.
^ permalink raw reply
* [PATCH v3 15/24] drm/i2c: tda998x: use irq for connection status and EDID read
From: Russell King - ARM Linux @ 2014-01-24 17:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E2A2E8.50806@gmail.com>
On Fri, Jan 24, 2014 at 06:29:12PM +0100, Sebastian Hesselbarth wrote:
> On 01/22/14 23:27, Russell King - ARM Linux wrote:
>> On Sun, Jan 19, 2014 at 07:58:43PM +0100, Jean-Francois Moine wrote:
>>> This patch adds the optional treatment of the tda998x IRQ.
>>>
>>> The interrupt function is used to know the display connection status
>>> without polling and to speedup reading the EDID.
>>>
>>> The interrupt number may be defined either in the DT or at encoder set
>>> config time for non-DT boards.
>>>
>>> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
>>> ---
> [...]
>>> @@ -720,6 +787,10 @@ tda998x_encoder_set_config(struct drm_encoder *encoder, void *params)
>>> priv->audio_port = p->audio_cfg;
>>> priv->audio_format = p->audio_format;
>>> }
>>> +
>>> + priv->irq = p->irq;
>>> + if (p->irq)
>>> + tda_irq_init(priv);
>>
>> If we're going to do it this way, this should probably release the IRQ if
>> there was one before re-claiming it, just in case this function gets called
>> more than once by some driver using it.
>>
>> The alternative is, as I said before, to use the infrastructure which is
>> already there, namely setting the interrupt via struct i2c_client's
>> irq member. Yes, that doesn't satisfy Sebastian's comment about using
>> a GPIO, but there's no sign of GPIO usage in here at the moment anyway.
>> So we might as well use what's already provided.
>
> Russell,
>
> I am fine with using an irq instead of gpio here. I remember you telling
> me on a similar patch, that from the gpio you can derive the irq but
> not the other way round. Anyway, I also remember reading discussions
> about DT gpios vs interrupts, and IIRC the outcome was that passing
> interrupts is fine, too.
Sebastian,
You can derive the irq from a gpio (using gpio_to_irq()), but you can't
derive a gpio from an irq.
It's annoying that i2c_client and the i2c board data do not allow a GPIO
to be specified, but only an IRQ, but that's a separate problem which when
solved will also get solved here. So, I'm not too worried about the
"should this be an IRQ or should it be a GPIO" question here.
For us on ARM, it's less of a problem because we can just deal with GPIOs
or IRQs at the DT level, and so we don't care what's in i2c_client.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* [PATCH V2 4/6] ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS
From: Christopher Covington @ 2014-01-24 17:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124171754.GM31040@mudshark.cambridge.arm.com>
On 01/24/2014 12:17 PM, Will Deacon wrote:
> On Fri, Jan 24, 2014 at 05:16:28PM +0000, Adrien Verg? wrote:
>> 2014/1/24 Will Deacon <will.deacon@arm.com>:
>>> Are you sure about this? The value we write is actually task_pid_nr, which I
>>> believe to be globally unique.
>>
>> You are right: the task_pid_nr is unique in the system. However when
>> using namespaces, the so called "PID" is the virtual number that
>> processes in different namespaces can share.
>>
>> This PID is the one visible by user-space tasks, in particular
>> user-space tracers and debuggers. These programs would expect to find
>> the PID of the traced process in the Context ID reg, while it is not.
>> I think it is better to remove confusion by making PID_IN_CONTEXTIDR
>> and PID_NS incompatible.
>>
>> What do you think?
>
> I think I'd rather have the global ID than disable a potentially useful
> feature, especially since this is likely to be consumed by external trace
> tools as opposed to user-space tasks.
We've discussed before that the ARM architecture doesn't say what should be
written to the CONTEXTIDR, so it's up to us to decide. Will has a use case
where the global PID is useful. Adrien's patches present a use case where I
think the virtual PID would be useful. I've done work in the past where
writing the process group ID was useful. Would it be reasonable to make what's
written to the CONTEXTIDR run-time configurable? If so, what would be the best
interface for configuring it?
Thanks,
Christopher
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.
^ permalink raw reply
* [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
From: Christopher Covington @ 2014-01-24 17:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124113920.GF814@e106331-lin.cambridge.arm.com>
On 01/24/2014 06:39 AM, Mark Rutland wrote:
> On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
>> Add documentation for generic SYSCON reboot driver.
>>
>> Signed-off-by: Feng Kan <fkan@apm.com>
>> ---
>> .../bindings/power/reset/syscon-reboot.txt | 16 ++++++++++++++++
>> 1 files changed, 16 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> new file mode 100644
>> index 0000000..e9eb1fe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> @@ -0,0 +1,16 @@
>> +Generic SYSCON mapped register reset driver
>
> Bindings should describe hardware, not drivers.
How is this different than what's done for PSCI?
Thanks,
Christopher
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.
^ permalink raw reply
* [PATCH RFC v2 2/2] Documentation: arm: define DT C-states bindings
From: Lorenzo Pieralisi @ 2014-01-24 17:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKfTPtDHTs67qvoA6++zo4SZdZJj4T_BV2gVECOu-76-2kLbuw@mail.gmail.com>
Hi Vincent,
On Fri, Jan 24, 2014 at 08:40:40AM +0000, Vincent Guittot wrote:
[...]
> Hi Lorenzo,
>
> Sorry for the late reply,
>
>
> > I had an idea. To simplify things, I think that one possibility is to
> > add a parameter to the power domain specifier (platform specific, see
> > Tomasz bindings):
>
> We can't use a simple boolean state (on/off) for defining the
> powerdomain state associated to a c-state so your proposal of being
> able to add a parameter that will define the power domain state is
> interesting.
>
> >
> > Documentation/devicetree/bindings/power/power_domain.txt
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-January/224928.html
> >
> > to represent, when that state is entered the behavior of the power
> > controller (ie cache RAM retention or cache shutdown or in general any
> > substate within a power domain). Since it is platform specific, and since
> > we are able to link caches to the power domain, the power controller will
> > actually define what happens to the cache when that state is entered
> > (basically we use the power domain specifier additional parameter to define
> > a "substate" in that power domain e.g.:
> >
> > Example:
> >
> > foo_power_controller {
> > [...]
> > /*
> > * first cell is register index, second one is the state index
> > * that in turn implies the state behavior - eg cache lost or
> > * retained
> > */
> > #power-domain-cells = <2>;
> > };
> >
> > l1-cache {
> > [...]
> > /*
> > * syntax: power-domains = list of power domain specifiers
> > <[&power_domain_phandle register-index state],[&power_domain_phandle register-index state]>;
> > The syntax is defined by the power controller du jour
> > as described by Tomasz bindings
> > */
> > power-domains =<&foo_power_controller 0 0 &foo_power_controller 0 1>;
>
> Normally, power-domains describes a list of power domain specifiers
> that are necessary for the l1-cache to at least retain its state so
> i'm not sure understand your example above
>
> If we take the example of system that support running, retention and
> powerdown state described as state 0, 1 and 2 for the power domain, i
> would have set the l1-cache like:
> power-domains =<&foo_power_controller 0 1>;
>
> for saying that the state is retained up to state 1
>
> Please look below, i have modified the rest of your example accordingly
>
> >
> > }:
> >
> > and then
> >
> > state0 {
> > index = <2>;
> > compatible = "arm,cpu-power-state";
> > latency = <...>;
> > /*
> > * This means that when the state is entered, the power
> > * controller should use register index 0 and state 0,
> > * whose meaning is power controller specific. Since we
> > * know all components affected (for every component
> > * we declare its power domain(s) and states so we
> > * know what components are affected by the state entry.
> > * Given the cache node above and this phandle, the state
> > * implies that the cache is retained, register index == 0 state == 0
> > /*
> > power-domain =<&foo_power_controller 0 0>;
>
> for retention state we need to set the power domain in state 1
> power-domain =<&foo_power_controller 0 1>;
>
> > };
> >
> > state1 {
> > index = <3>;
> > compatible = "arm,cpu-power-state";
> > latency = <...>;
> > /*
> > * This means that when the state is entered, the power
> > * controller should use register index 0 and state 1,
> > * whose meaning is power controller specific. Since we
> > * know all components affected (for every component
> > * we declare its power domain(s) and states so we
> > * know what components are affected by the state entry.
> > * Given the cache node above and this phandle, the state
> > * implies that the cache is lost, register index == 0 state == 1
> > /*
> > power-domain =<&foo_power_controller 0 1>;
>
> for power down mode, we need to set thge power domain in state 2
> power-domain =<&foo_power_controller 0 2>;
Ok, what I meant was not what you got, but your approach looks sensible
too. What I do not like is that the power-domain specifier is power
controller specific (that was true even for my example). In theory
we can achieve something identical by forcing every component in a power
domain to specify the max C-state index that allows it to retain its
state (through a specific property). Same logic to your example applies.
Nice thing is that we do not change the power domain specifiers, bad thing
is that it adds two properties to each device (c-state index and
power-domain-specifier - but we can make it hierarchical so that device
nodes can inherit the maximum operating C-state by inheriting the value
from a parent node providing a common value).
In my example the third parameter was just a number that the power
controller would decode (eg 0 = cache retained, 1 = cache lost)
according to its implementation, it was not a "state index". The
power controller would know what to do with eg a cache component (that
declares to be in that power domain) when a C-state with that power
domain specifier was entered.
Not very different from what you are saying, let's get to the nub:
- Either we define it in a platform specific way through the power
domain specifier
- Or we force a max-c-state-supported property for every device,
possibly hierarchical
Thoughts ?
Thank you !
Lorenzo
^ permalink raw reply
* [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
From: Feng Kan @ 2014-01-24 18:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124113920.GF814@e106331-lin.cambridge.arm.com>
On Fri, Jan 24, 2014 at 3:39 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
>> Add documentation for generic SYSCON reboot driver.
>>
>> Signed-off-by: Feng Kan <fkan@apm.com>
>> ---
>> .../bindings/power/reset/syscon-reboot.txt | 16 ++++++++++++++++
>> 1 files changed, 16 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> new file mode 100644
>> index 0000000..e9eb1fe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> @@ -0,0 +1,16 @@
>> +Generic SYSCON mapped register reset driver
>
> Bindings should describe hardware, not drivers.
>
> What precisely does this binding describe?
>
>> +
>> +Required properties:
>> +- compatible: should contain "syscon-reboot"
>> +- regmap: this is phandle to the register map node
>> +- offset: offset in the register map for the reboot register
>> +- mask: the reset value written to the reboot register
>> +
>> +Examples:
>> +
>> +reboot {
>> + compatible = "syscon-reboot";
>> + regmap = <®mapnode>;
>> + offset = <0x0>;
>> + mask = <0x1>;
>> +};
>
> Access size? Endianness?
FKAN: are you asking for documentation? I don't see alot of example of
support for these.
>
> Why can we not have a binding for the register bank this exists in, and
> have that pass on the appropriate details to a syscon-reboot driver?
FKAN: Thats a good idea. But the hardware in this case (SCU) system
clock unit has a bunch of registers used for different functions. If syscon is
used alot in this case and we pile more attribute into it. It would get kinda
messy after a while.
FKAN: I still haven't figured out how to generically tie to
the reset handler? Maybe the next person can use #define to bridge in the
reset handler they want to use.
>
> That way we can change the way we poke things without requiring changes
> to bindings or dts.
>
> Thanks,
> Mark.
^ permalink raw reply
* [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
From: Mark Rutland @ 2014-01-24 18:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E2A901.4050707@codeaurora.org>
On Fri, Jan 24, 2014 at 05:55:13PM +0000, Christopher Covington wrote:
> On 01/24/2014 06:39 AM, Mark Rutland wrote:
> > On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
> >> Add documentation for generic SYSCON reboot driver.
> >>
> >> Signed-off-by: Feng Kan <fkan@apm.com>
> >> ---
> >> .../bindings/power/reset/syscon-reboot.txt | 16 ++++++++++++++++
> >> 1 files changed, 16 insertions(+), 0 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >> new file mode 100644
> >> index 0000000..e9eb1fe
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >> @@ -0,0 +1,16 @@
> >> +Generic SYSCON mapped register reset driver
> >
> > Bindings should describe hardware, not drivers.
>
> How is this different than what's done for PSCI?
A PSCI node in the DT defines a standard interface to a firmware which
is external to Linux. The PSCI binding does not contain the word
"driver", but describes the interface that the binding describes, with
reference to approriate documentation.
All I'm arguing for here is a description of the class of hardware this
is applicable to, rather than "this is what this particular driver
uses".
Thanks,
Mark.
^ permalink raw reply
* [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
From: Mark Rutland @ 2014-01-24 18:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL85gmDJeUY0zUjkmpMo7g_hLj34RsgU1VEF80wouLzOFGAdqQ@mail.gmail.com>
On Fri, Jan 24, 2014 at 06:03:10PM +0000, Feng Kan wrote:
> On Fri, Jan 24, 2014 at 3:39 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
> >> Add documentation for generic SYSCON reboot driver.
> >>
> >> Signed-off-by: Feng Kan <fkan@apm.com>
> >> ---
> >> .../bindings/power/reset/syscon-reboot.txt | 16 ++++++++++++++++
> >> 1 files changed, 16 insertions(+), 0 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >> new file mode 100644
> >> index 0000000..e9eb1fe
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> >> @@ -0,0 +1,16 @@
> >> +Generic SYSCON mapped register reset driver
> >
> > Bindings should describe hardware, not drivers.
> >
> > What precisely does this binding describe?
> >
> >> +
> >> +Required properties:
> >> +- compatible: should contain "syscon-reboot"
> >> +- regmap: this is phandle to the register map node
> >> +- offset: offset in the register map for the reboot register
> >> +- mask: the reset value written to the reboot register
> >> +
> >> +Examples:
> >> +
> >> +reboot {
> >> + compatible = "syscon-reboot";
> >> + regmap = <®mapnode>;
> >> + offset = <0x0>;
> >> + mask = <0x1>;
> >> +};
> >
> > Access size? Endianness?
> FKAN: are you asking for documentation? I don't see alot of example of
> support for these.
If I used the enippet in the example, what endianness and access size
should I expect an OS to perform? That should be documented.
If this doesn't match the general case, we can add properties later to
adjust the access size and/or endianness. We just need to document what
the binding actually describes currently, or it's not possible to
implement anything based off of the binding documentation.
I should be able to read a binding document and write a dts. I shouldn't
have to read the code to figure out what the binding describes.
>
> >
> > Why can we not have a binding for the register bank this exists in, and
> > have that pass on the appropriate details to a syscon-reboot driver?
>
> FKAN: Thats a good idea. But the hardware in this case (SCU) system
> clock unit has a bunch of registers used for different functions. If syscon is
> used alot in this case and we pile more attribute into it. It would get kinda
> messy after a while.
Huh?
What's wrong with having a system clock unit binding, that the kernel
can decompose as appropriate?
I don't get your syscon argument.
Thanks,
Mark.
^ permalink raw reply
* [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
From: Feng Kan @ 2014-01-24 18:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124182355.GF4758@e106331-lin.cambridge.arm.com>
On Fri, Jan 24, 2014 at 10:23 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Jan 24, 2014 at 06:03:10PM +0000, Feng Kan wrote:
>> On Fri, Jan 24, 2014 at 3:39 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
>> >> Add documentation for generic SYSCON reboot driver.
>> >>
>> >> Signed-off-by: Feng Kan <fkan@apm.com>
>> >> ---
>> >> .../bindings/power/reset/syscon-reboot.txt | 16 ++++++++++++++++
>> >> 1 files changed, 16 insertions(+), 0 deletions(-)
>> >> create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> >> new file mode 100644
>> >> index 0000000..e9eb1fe
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> >> @@ -0,0 +1,16 @@
>> >> +Generic SYSCON mapped register reset driver
>> >
>> > Bindings should describe hardware, not drivers.
>> >
>> > What precisely does this binding describe?
>> >
>> >> +
>> >> +Required properties:
>> >> +- compatible: should contain "syscon-reboot"
>> >> +- regmap: this is phandle to the register map node
>> >> +- offset: offset in the register map for the reboot register
>> >> +- mask: the reset value written to the reboot register
>> >> +
>> >> +Examples:
>> >> +
>> >> +reboot {
>> >> + compatible = "syscon-reboot";
>> >> + regmap = <®mapnode>;
>> >> + offset = <0x0>;
>> >> + mask = <0x1>;
>> >> +};
>> >
>> > Access size? Endianness?
>> FKAN: are you asking for documentation? I don't see alot of example of
>> support for these.
>
> If I used the enippet in the example, what endianness and access size
> should I expect an OS to perform? That should be documented.
>
> If this doesn't match the general case, we can add properties later to
> adjust the access size and/or endianness. We just need to document what
> the binding actually describes currently, or it's not possible to
> implement anything based off of the binding documentation.
>
> I should be able to read a binding document and write a dts. I shouldn't
> have to read the code to figure out what the binding describes.
>
>>
>> >
>> > Why can we not have a binding for the register bank this exists in, and
>> > have that pass on the appropriate details to a syscon-reboot driver?
>>
>> FKAN: Thats a good idea. But the hardware in this case (SCU) system
>> clock unit has a bunch of registers used for different functions. If syscon is
>> used alot in this case and we pile more attribute into it. It would get kinda
>> messy after a while.
>
> Huh?
>
> What's wrong with having a system clock unit binding, that the kernel
> can decompose as appropriate?
>
> I don't get your syscon argument.
FKAN: I do have a SCU binding, I thought you wanted to move the offset and
mask to the SCU binding. The only issue I see in that case is when we use
more such methods, the SCU binding would look rather crowded. If this is not
the case, I am a bit confused at what I should do next.
>
> Thanks,
> Mark.
^ permalink raw reply
* [RESEND PATCH] ARM: dts: bcm28155-ap: Fix Card Detection GPIO
From: Tim Kryger @ 2014-01-24 18:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAEPRUYpyr2_3pfa3rqYiYPAOwJfA+0AdqyGaf5s7YiWH=zfTeA@mail.gmail.com>
On Wed, Jan 8, 2014 at 4:54 PM, Christian Daudt <bcm@fixthebug.org> wrote:
> On Wed, Jan 8, 2014 at 4:28 PM, Tim Kryger <tim.kryger@linaro.org> wrote:
>> On Wed, Jan 8, 2014 at 3:38 PM, Christian Daudt <bcm@fixthebug.org> wrote:
>>> On Tue, Jan 7, 2014 at 10:53 AM, Tim Kryger <tim.kryger@linaro.org> wrote:
>>>> The board schematic states that the "SD_CARD_DET_N gets pulled to GND
>>>> when card is inserted" so the polarity has been updated to active low.
>>>>
>>>> Polarity is now specified with a GPIO define instead of a magic number.
>>>>
>>>> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
>>>> Reviewed-by: Matt Porter <matt.porter@linaro.org>
>>>> ---
>>>> arch/arm/boot/dts/bcm28155-ap.dts | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/bcm28155-ap.dts b/arch/arm/boot/dts/bcm28155-ap.dts
>>>> index 08e47c2..27dd110 100644
>>>> --- a/arch/arm/boot/dts/bcm28155-ap.dts
>>>> +++ b/arch/arm/boot/dts/bcm28155-ap.dts
>>>> @@ -13,6 +13,8 @@
>>>>
>>>> /dts-v1/;
>>>>
>>>> +#include <dt-bindings/gpio/gpio.h>
>>>> +
>>>> #include "bcm11351.dtsi"
>>>>
>>>> / {
>>>> @@ -40,7 +42,7 @@
>>>>
>>>> sdio4: sdio at 3f1b0000 {
>>>> max-frequency = <48000000>;
>>>> - cd-gpios = <&gpio 14 0>;
>>>> + cd-gpios = <&gpio 14 GPIO_ACTIVE_LOW>;
>>>> status = "okay";
>>>> };
>>>> };
>>>> --
>>>> 1.8.0.1
>>>>
>>> Tim,
>>> Does bcm11351-brt not also suffer from the same bug? If it does can
>>> you pls update the patch to also fix it?
>>>
>>> Thanks,
>>> csd
>>
>> The BRT and AP boards are similar so it may have the same problem but
>> I don't have a BRT and wouldn't be able to test any changes to its DTS
>> file.
>>
>> -Tim
>
> [sorry for the resend for those that get it twice]
> Agreed - it's time that dts file go away. In this case:
> Acked-by: Christian Daudt <bcm@fixthebug.org>
>
> Olof - can you pls pull in this patch. This is the bugfix that was
> discussed in irc earlier today.
>
> thanks,
> csd
Christian,
I'm not sure Olof saw your reply. Olof and Kevin are now on CC.
Thanks,
Tim Kryger
^ permalink raw reply
* [PATCH V2 4/6] ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS
From: Christopher Covington @ 2014-01-24 19:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E2A866.1050400@codeaurora.org>
On 01/24/2014 12:52 PM, Christopher Covington wrote:
> On 01/24/2014 12:17 PM, Will Deacon wrote:
>> On Fri, Jan 24, 2014 at 05:16:28PM +0000, Adrien Verg? wrote:
>>> 2014/1/24 Will Deacon <will.deacon@arm.com>:
>>>> Are you sure about this? The value we write is actually task_pid_nr, which I
>>>> believe to be globally unique.
>>>
>>> You are right: the task_pid_nr is unique in the system. However when
>>> using namespaces, the so called "PID" is the virtual number that
>>> processes in different namespaces can share.
>>>
>>> This PID is the one visible by user-space tasks, in particular
>>> user-space tracers and debuggers. These programs would expect to find
>>> the PID of the traced process in the Context ID reg, while it is not.
>>> I think it is better to remove confusion by making PID_IN_CONTEXTIDR
>>> and PID_NS incompatible.
>>>
>>> What do you think?
>>
>> I think I'd rather have the global ID than disable a potentially useful
>> feature, especially since this is likely to be consumed by external trace
>> tools as opposed to user-space tasks.
>
> We've discussed before that the ARM architecture doesn't say what should be
> written to the CONTEXTIDR, so it's up to us to decide. Will has a use case
> where the global PID is useful. Adrien's patches present a use case where I
> think the virtual PID would be useful. I've done work in the past where
> writing the process group ID was useful. Would it be reasonable to make what's
> written to the CONTEXTIDR run-time configurable? If so, what would be the best
> interface for configuring it?
D'oh, I mixed things up. For ETM to work it can only use global PID's in the
CONTEXTIDR.
Christopher
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.
^ permalink raw reply
* [PATCH V2 4/6] ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS
From: Adrien Vergé @ 2014-01-24 19:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E2A866.1050400@codeaurora.org>
2014/1/24 Will Deacon <will.deacon@arm.com>:
> I think I'd rather have the global ID than disable a potentially useful
> feature, especially since this is likely to be consumed by external trace
> tools as opposed to user-space tasks.
I understand.
2014/1/24 Christopher Covington <cov@codeaurora.org>:
> Would it be reasonable to make what's
> written to the CONTEXTIDR run-time configurable? If so, what would be the best
> interface for configuring it?
This is an interesting option.
An other option would be to keep the global PID in the Context ID
register, and rely on kernel support to translate virtual PID to
global PID when needed. Then, it would be possible to select a task to
trace via its PID, by asking the kernel to write its global ID to the
Context ID comparator.
^ permalink raw reply
* [PATCH] ARM: dts: imx28-m28cu3: Remove 'reset-active-high'
From: Fabio Estevam @ 2014-01-24 20:45 UTC (permalink / raw)
To: linux-arm-kernel
From: Fabio Estevam <fabio.estevam@freescale.com>
The 'reset-active-high' property is not defined anywhere, so just remove it.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
arch/arm/boot/dts/imx28-m28cu3.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx28-m28cu3.dts b/arch/arm/boot/dts/imx28-m28cu3.dts
index d3958da..4174bb1 100644
--- a/arch/arm/boot/dts/imx28-m28cu3.dts
+++ b/arch/arm/boot/dts/imx28-m28cu3.dts
@@ -116,7 +116,6 @@
pinctrl-0 = <&lcdif_24bit_pins_a
&lcdif_pins_m28>;
display = <&display>;
- reset-active-high;
status = "okay";
display: display0 {
--
1.8.1.2
^ permalink raw reply related
* [PATCH V2 4/8] SPEAr13xx: Fixup: Move SPEAr1340 SATA platform code to system cfg driver
From: Arnd Bergmann @ 2014-01-24 20:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124042959.GC2369@pratyush-vbox>
On Friday 24 January 2014, Pratyush Anand wrote:
> On Thu, Jan 23, 2014 at 08:22:54PM +0800, Arnd Bergmann wrote:
> > On Thursday 23 January 2014, Mohit Kumar wrote:
> >
> > I assume you'd want a phandle pointing to the syscon device in here
> > as well?
>
> Since there is only one syscon device in the whole DT, so do I really
> need to add phandle? Currently I am using
> syscon_regmap_lookup_by_compatible to find syscon device.
I'd much rather use syscon_regmap_lookup_by_phandle than
syscon_regmap_lookup_by_compatible, all the time, since this makes
the relationship between the devices explicit.
The phandle method also allows you to pass regmap indexes in the
same property, which can be handy if two variants of the chip have
the same registers at a different offset.
> > > +/* SPEAr1340 Registers */
> > > +/* Power Management Registers */
> > > +#define SPEAR1340_PCM_CFG 0x100
> > > + #define SPEAR1340_PCM_CFG_SATA_POWER_EN 0x800
> > > +#define SPEAR1340_PCM_WKUP_CFG 0x104
> > > +#define SPEAR1340_SWITCH_CTR 0x108
> > > +
> > > +#define SPEAR1340_PERIP1_SW_RST 0x318
> > > + #define SPEAR1340_PERIP1_SW_RST_SATA 0x1000
> > > +#define SPEAR1340_PERIP2_SW_RST 0x31C
> > > +#define SPEAR1340_PERIP3_SW_RST 0x320
> > > +
> > > +/* PCIE - SATA configuration registers */
> > > +#define SPEAR1340_PCIE_SATA_CFG 0x424
> > > + /* PCIE CFG MASks */
> > > + #define SPEAR1340_PCIE_CFG_DEVICE_PRESENT (1 << 11)
> > > + #define SPEAR1340_PCIE_CFG_POWERUP_RESET (1 << 10)
> > > + #define SPEAR1340_PCIE_CFG_CORE_CLK_EN (1 << 9)
> > > + #define SPEAR1340_PCIE_CFG_AUX_CLK_EN (1 << 8)
> > > + #define SPEAR1340_SATA_CFG_TX_CLK_EN (1 << 4)
> > > + #define SPEAR1340_SATA_CFG_RX_CLK_EN (1 << 3)
> > > + #define SPEAR1340_SATA_CFG_POWERUP_RESET (1 << 2)
> > > + #define SPEAR1340_SATA_CFG_PM_CLK_EN (1 << 1)
> > > + #define SPEAR1340_PCIE_SATA_SEL_PCIE (0)
> > > + #define SPEAR1340_PCIE_SATA_SEL_SATA (1)
> > > + #define SPEAR1340_PCIE_SATA_CFG_MASK 0xF1F
> > > + #define SPEAR1340_PCIE_CFG_VAL (SPEAR1340_PCIE_SATA_SEL_PCIE | \
> > > + SPEAR1340_PCIE_CFG_AUX_CLK_EN | \
> > > + SPEAR1340_PCIE_CFG_CORE_CLK_EN | \
> > > + SPEAR1340_PCIE_CFG_POWERUP_RESET | \
> > > + SPEAR1340_PCIE_CFG_DEVICE_PRESENT)
> > > + #define SPEAR1340_SATA_CFG_VAL (SPEAR1340_PCIE_SATA_SEL_SATA | \
> > > + SPEAR1340_SATA_CFG_PM_CLK_EN | \
> > > + SPEAR1340_SATA_CFG_POWERUP_RESET | \
> > > + SPEAR1340_SATA_CFG_RX_CLK_EN | \
> > > + SPEAR1340_SATA_CFG_TX_CLK_EN)
> > > +
> > > +#define SPEAR1340_PCIE_MIPHY_CFG 0x428
> > > + #define SPEAR1340_MIPHY_OSC_BYPASS_EXT (1 << 31)
> > > + #define SPEAR1340_MIPHY_CLK_REF_DIV2 (1 << 27)
> > > + #define SPEAR1340_MIPHY_CLK_REF_DIV4 (2 << 27)
> > > + #define SPEAR1340_MIPHY_CLK_REF_DIV8 (3 << 27)
> > > + #define SPEAR1340_MIPHY_PLL_RATIO_TOP(x) (x << 0)
> > > + #define SPEAR1340_PCIE_MIPHY_CFG_MASK 0xF80000FF
> > > + #define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA \
> > > + (SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
> > > + SPEAR1340_MIPHY_CLK_REF_DIV2 | \
> > > + SPEAR1340_MIPHY_PLL_RATIO_TOP(60))
> > > + #define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK \
> > > + (SPEAR1340_MIPHY_PLL_RATIO_TOP(120))
> > > + #define SPEAR1340_PCIE_SATA_MIPHY_CFG_PCIE \
> > > + (SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
> > > + SPEAR1340_MIPHY_PLL_RATIO_TOP(25))
> > > +
> > > +struct spear13xx_cfg_priv {
> > > + struct regmap *misc;
> > > +};
> > > +
> > > +/* SATA device registration */
> > > +static void spear1340_sata_miphy_init(struct spear13xx_cfg_priv *cfgpriv)
> > > +{
> > > + regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_SATA_CFG,
> > > + SPEAR1340_PCIE_SATA_CFG_MASK, SPEAR1340_SATA_CFG_VAL);
> > > + regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_MIPHY_CFG,
> > > + SPEAR1340_PCIE_MIPHY_CFG_MASK,
> > > + SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK);
> > > + /* Switch on sata power domain */
> > > + regmap_update_bits(cfgpriv->misc, SPEAR1340_PCM_CFG,
> > > + SPEAR1340_PCM_CFG_SATA_POWER_EN,
> > > + SPEAR1340_PCM_CFG_SATA_POWER_EN);
> > > + msleep(20);
> > > + /* Disable PCIE SATA Controller reset */
> > > + regmap_update_bits(cfgpriv->misc, SPEAR1340_PERIP1_SW_RST,
> > > + SPEAR1340_PERIP1_SW_RST_SATA, 0);
> > > + msleep(20);
> > > +}
> >
> > Looking at the actual code now, this very much looks like it ought to
> > be a "phy" driver and get put in drivers/phy/.
>
> Actually these registers are part of common system configurations
> register space (called as misc space) for SPEAr SOC. So we opted for
> syscon framework.
The use of syscon for this is good, I have no objection to that, and
was suggesting that you create a logical "phy" device that uses the
misc syscon device as a backend.
> PHY registers space starts from 0xEB800000, which can be
> programmed for various phy specific functions like power management,
> tx/rx settings, comparator settings etc. In most of the cases phy
> works with default settings, however there are few exceptions for
> which we will be adding a phy driver for further improvement of SPEAr
> drivers.
I see. So while the code you have here could be expressed as a phy driver
by itself, there is another part of the SoC that controls the actual
phy. How about if you add the phy device node to DT, and write a driver
that doesn't actually program the phy registers for now, but does contain
the code that you have posted here. That would give you flexibility for
future extensions and at the same time let you remove all SPEAr specific
code from the actual AHCI driver by using the generic ahci-platform
driver.
Arnd
^ permalink raw reply
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