* [PATCH v2 3/4] ASoC: wm8904: automatically choose clock source
From: Michał Mirosław @ 2019-08-25 12:17 UTC (permalink / raw)
To: alsa-devel, patches
Cc: Kate Stewart, Maxime Jourdan, Alexandre Belloni,
Kuninori Morimoto, Kirill Marinushkin, Takashi Iwai,
Paul Cercueil, Srinivas Kandagatla, Jerome Brunet, Anders Roxell,
Ludovic Desroches, linux-arm-kernel, Codrin Ciubotariu,
Charles Keepax, Liam Girdwood, Piotr Stankiewicz,
Annaliese McDermond, Richard Fitzgerald, Mark Brown,
Nariman Poushin, Thomas Gleixner, Jaroslav Kysela, zhong jiang,
Allison Randal, Greg Kroah-Hartman, Randy Dunlap, Nikesh Oswal,
linux-kernel, Enrico Weigelt
In-Reply-To: <cover.1566734630.git.mirq-linux@rere.qmqm.pl>
Choose clock source automatically if not provided. This will be the case
with eg. audio-graph-card.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
sound/soc/codecs/wm8904.c | 42 +++++++++++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
index c9318fe34f91..946315d4cecf 100644
--- a/sound/soc/codecs/wm8904.c
+++ b/sound/soc/codecs/wm8904.c
@@ -367,15 +367,34 @@ static int wm8904_enable_sysclk(struct wm8904_priv *priv)
return err;
}
+static int wm8904_bump_fll_sysclk(unsigned int *rate);
+
static int wm8904_set_sysclk(struct snd_soc_dai *dai, int clk_id,
unsigned int rate, int dir)
{
struct snd_soc_component *component = dai->component;
struct wm8904_priv *wm8904 = snd_soc_component_get_drvdata(component);
unsigned int clock0, clock2;
- int err;
+ int err, do_div = false;
switch (clk_id) {
+ case 0:
+ if (rate == clk_round_rate(wm8904->mclk, rate)) {
+ clk_id = WM8904_CLK_MCLK;
+ } else if (rate * 2 == clk_round_rate(wm8904->mclk, rate * 2)) {
+ rate *= 2;
+ clk_id = WM8904_CLK_MCLK;
+ do_div = true;
+ } else {
+ clk_id = WM8904_CLK_FLL;
+ err = wm8904_bump_fll_sysclk(&rate);
+ if (err) {
+ dev_dbg(component->dev, "Can't match %u over FLL 1406250 Hz minimum\n", rate);
+ return err;
+ }
+ }
+ break;
+
case WM8904_CLK_MCLK:
case WM8904_CLK_FLL:
break;
@@ -421,7 +440,9 @@ static int wm8904_set_sysclk(struct snd_soc_dai *dai, int clk_id,
}
/* SYSCLK shouldn't be over 13.5MHz */
- if (rate > 13500000) {
+ if (rate > 13500000)
+ do_div = true;
+ if (do_div) {
clock0 = WM8904_MCLK_DIV;
wm8904->sysclk_rate = rate / 2;
} else {
@@ -1350,6 +1371,23 @@ static struct {
{ 480, 20 },
};
+static int wm8904_bump_fll_sysclk(unsigned int *rate)
+{
+ int i;
+
+ /* bump SYSCLK rate if below minimal FLL output */
+
+ for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
+ if (*rate * bclk_divs[i].div >= 1406250 * 10)
+ break;
+ }
+
+ if (i == ARRAY_SIZE(bclk_divs))
+ return -ERANGE;
+
+ *rate = (*rate * bclk_divs[i].div) / 10;
+ return 0;
+}
static int wm8904_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
--
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
* [PATCH v2 4/4] [RFT] ASoC: wm8994: use common FLL code
From: Michał Mirosław @ 2019-08-25 12:17 UTC (permalink / raw)
To: alsa-devel, patches
Cc: Maxime Jourdan, Kate Stewart, Alexandre Belloni,
Kuninori Morimoto, Kirill Marinushkin, Liam Girdwood,
Paul Cercueil, Srinivas Kandagatla, Jerome Brunet, Anders Roxell,
Ludovic Desroches, linux-arm-kernel, Codrin Ciubotariu,
Charles Keepax, Piotr Stankiewicz, Nariman Poushin,
Richard Fitzgerald, Mark Brown, Annaliese McDermond,
Thomas Gleixner, Jaroslav Kysela, zhong jiang, Allison Randal,
Greg Kroah-Hartman, Randy Dunlap, Nikesh Oswal, Takashi Iwai,
linux-kernel, Enrico Weigelt
In-Reply-To: <cover.1566734630.git.mirq-linux@rere.qmqm.pl>
Rework FLL handling to use common code.
This uses polling for now to wait for FLL lock.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
sound/soc/codecs/Kconfig | 2 +
sound/soc/codecs/wm8994.c | 281 +++++++++++---------------------------
sound/soc/codecs/wm8994.h | 4 +-
3 files changed, 84 insertions(+), 203 deletions(-)
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 1a680023af7d..1ff6290ce18d 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -1382,6 +1382,8 @@ config SND_SOC_WM8993
config SND_SOC_WM8994
tristate
+ select SND_SOC_WM_FLL
+ select SND_SOC_WM_FLL_EFS
config SND_SOC_WM8995
tristate
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index c3d06e8bc54f..d0dbc352303b 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -2030,101 +2030,57 @@ static const struct snd_soc_dapm_route wm8958_intercon[] = {
{ "AIF3ADC Mux", "Mono PCM", "Mono PCM Out Mux" },
};
-/* The size in bits of the FLL divide multiplied by 10
- * to allow rounding later */
-#define FIXED_FLL_SIZE ((1 << 16) * 10)
-
-struct fll_div {
- u16 outdiv;
- u16 n;
- u16 k;
- u16 lambda;
- u16 clk_ref_div;
- u16 fll_fratio;
+static const struct wm_fll_desc wm8994_fll_desc[2] = {
+ /* FLL1 */
+ {
+ .ctl_offset = WM8994_FLL1_CONTROL_1,
+ .int_offset = WM8994_INTERRUPT_RAW_STATUS_2,
+ .int_mask = WM8994_IM_FLL1_LOCK_EINT_MASK,
+ .nco_reg0 = WM8994_FLL1_CONTROL_5,
+ .frc_nco_shift = 6,
+ .nco_reg1 = WM8994_FLL1_CONTROL_5,
+ .frc_nco_val_shift = 7,
+ .clk_ref_map = { FLL_REF_MCLK, FLL_REF_MCLK2, FLL_REF_FSCLK, FLL_REF_BCLK },
+ },
+ /* FLL2 */
+ {
+ .ctl_offset = WM8994_FLL2_CONTROL_1,
+ .int_offset = WM8994_INTERRUPT_RAW_STATUS_2,
+ .int_mask = WM8994_IM_FLL2_LOCK_EINT_MASK,
+ .nco_reg0 = WM8994_FLL2_CONTROL_5,
+ .frc_nco_shift = 6,
+ .nco_reg1 = WM8994_FLL2_CONTROL_5,
+ .frc_nco_val_shift = 7,
+ .clk_ref_map = { FLL_REF_MCLK, FLL_REF_MCLK2, FLL_REF_FSCLK, FLL_REF_BCLK },
+ },
};
-static int wm8994_get_fll_config(struct wm8994 *control, struct fll_div *fll,
- int freq_in, int freq_out)
-{
- u64 Kpart;
- unsigned int K, Ndiv, Nmod, gcd_fll;
-
- pr_debug("FLL input=%dHz, output=%dHz\n", freq_in, freq_out);
-
- /* Scale the input frequency down to <= 13.5MHz */
- fll->clk_ref_div = 0;
- while (freq_in > 13500000) {
- fll->clk_ref_div++;
- freq_in /= 2;
-
- if (fll->clk_ref_div > 3)
- return -EINVAL;
- }
- pr_debug("CLK_REF_DIV=%d, Fref=%dHz\n", fll->clk_ref_div, freq_in);
-
- /* Scale the output to give 90MHz<=Fvco<=100MHz */
- fll->outdiv = 3;
- while (freq_out * (fll->outdiv + 1) < 90000000) {
- fll->outdiv++;
- if (fll->outdiv > 63)
- return -EINVAL;
- }
- freq_out *= fll->outdiv + 1;
- pr_debug("OUTDIV=%d, Fvco=%dHz\n", fll->outdiv, freq_out);
-
- if (freq_in > 1000000) {
- fll->fll_fratio = 0;
- } else if (freq_in > 256000) {
- fll->fll_fratio = 1;
- freq_in *= 2;
- } else if (freq_in > 128000) {
- fll->fll_fratio = 2;
- freq_in *= 4;
- } else if (freq_in > 64000) {
- fll->fll_fratio = 3;
- freq_in *= 8;
- } else {
- fll->fll_fratio = 4;
- freq_in *= 16;
- }
- pr_debug("FLL_FRATIO=%d, Fref=%dHz\n", fll->fll_fratio, freq_in);
-
- /* Now, calculate N.K */
- Ndiv = freq_out / freq_in;
-
- fll->n = Ndiv;
- Nmod = freq_out % freq_in;
- pr_debug("Nmod=%d\n", Nmod);
-
- switch (control->type) {
- case WM8994:
- /* Calculate fractional part - scale up so we can round. */
- Kpart = FIXED_FLL_SIZE * (long long)Nmod;
-
- do_div(Kpart, freq_in);
-
- K = Kpart & 0xFFFFFFFF;
-
- if ((K % 10) >= 5)
- K += 5;
-
- /* Move down to proper range now rounding is done */
- fll->k = K / 10;
- fll->lambda = 0;
-
- pr_debug("N=%x K=%x\n", fll->n, fll->k);
- break;
-
- default:
- gcd_fll = gcd(freq_out, freq_in);
-
- fll->k = (freq_out - (freq_in * fll->n)) / gcd_fll;
- fll->lambda = freq_in / gcd_fll;
-
- }
-
- return 0;
-}
+static const struct wm_fll_desc wm8958_fll_desc[2] = {
+ /* FLL1 */
+ {
+ .ctl_offset = WM8994_FLL1_CONTROL_1,
+ .int_offset = WM8994_INTERRUPT_RAW_STATUS_2,
+ .int_mask = WM8994_IM_FLL1_LOCK_EINT_MASK,
+ .nco_reg0 = WM8994_FLL1_CONTROL_5,
+ .frc_nco_shift = 6,
+ .nco_reg1 = WM8994_FLL1_CONTROL_5,
+ .frc_nco_val_shift = 7,
+ .efs_offset = WM8958_FLL1_EFS_1,
+ .clk_ref_map = { FLL_REF_MCLK, FLL_REF_MCLK2, FLL_REF_FSCLK, FLL_REF_BCLK },
+ },
+ /* FLL2 */
+ {
+ .ctl_offset = WM8994_FLL2_CONTROL_1,
+ .int_offset = WM8994_INTERRUPT_RAW_STATUS_2,
+ .int_mask = WM8994_IM_FLL2_LOCK_EINT_MASK,
+ .nco_reg0 = WM8994_FLL2_CONTROL_5,
+ .frc_nco_shift = 6,
+ .nco_reg1 = WM8994_FLL2_CONTROL_5,
+ .frc_nco_val_shift = 7,
+ .efs_offset = WM8958_FLL1_EFS_1,
+ .clk_ref_map = { FLL_REF_MCLK, FLL_REF_MCLK2, FLL_REF_FSCLK, FLL_REF_BCLK },
+ },
+};
static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
unsigned int freq_in, unsigned int freq_out)
@@ -2132,9 +2088,7 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
struct wm8994_priv *wm8994 = snd_soc_component_get_drvdata(component);
struct wm8994 *control = wm8994->wm8994;
int reg_offset, ret;
- struct fll_div fll;
u16 reg, clk1, aif_reg, aif_src;
- unsigned long timeout;
bool was_enabled;
switch (id) {
@@ -2152,9 +2106,6 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
return -EINVAL;
}
- reg = snd_soc_component_read32(component, WM8994_FLL1_CONTROL_1 + reg_offset);
- was_enabled = reg & WM8994_FLL1_ENA;
-
switch (src) {
case 0:
/* Allow no source specification when stopping */
@@ -2166,10 +2117,12 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
case WM8994_FLL_SRC_MCLK2:
case WM8994_FLL_SRC_LRCLK:
case WM8994_FLL_SRC_BCLK:
+ src = wm8994_fll_desc[0].clk_ref_map[src - WM8994_FLL_SRC_MCLK1];
break;
case WM8994_FLL_SRC_INTERNAL:
freq_in = 12000000;
freq_out = 12000000;
+ src = FLL_REF_OSC;
break;
default:
return -EINVAL;
@@ -2180,18 +2133,6 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
wm8994->fll[id].in == freq_in && wm8994->fll[id].out == freq_out)
return 0;
- /* If we're stopping the FLL redo the old config - no
- * registers will actually be written but we avoid GCC flow
- * analysis bugs spewing warnings.
- */
- if (freq_out)
- ret = wm8994_get_fll_config(control, &fll, freq_in, freq_out);
- else
- ret = wm8994_get_fll_config(control, &fll, wm8994->fll[id].in,
- wm8994->fll[id].out);
- if (ret < 0)
- return ret;
-
/* Make sure that we're not providing SYSCLK right now */
clk1 = snd_soc_component_read32(component, WM8994_CLOCKING_1);
if (clk1 & WM8994_SYSCLK_SRC)
@@ -2207,9 +2148,11 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
return -EBUSY;
}
- /* We always need to disable the FLL while reconfiguring */
- snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_1 + reg_offset,
- WM8994_FLL1_ENA, 0);
+ was_enabled = wm_fll_is_enabled(&wm8994->fll_hw[id]) > 0;
+
+ ret = wm_fll_disable(&wm8994->fll_hw[id]);
+ if (ret)
+ return ret;
if (wm8994->fll_byp && src == WM8994_FLL_SRC_BCLK &&
freq_in == freq_out && freq_out) {
@@ -2217,46 +2160,21 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_5 + reg_offset,
WM8958_FLL1_BYP, WM8958_FLL1_BYP);
goto out;
+ } else if (wm8994->fll_byp) {
+ snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_5 + reg_offset,
+ WM8958_FLL1_BYP, 0);
}
- reg = (fll.outdiv << WM8994_FLL1_OUTDIV_SHIFT) |
- (fll.fll_fratio << WM8994_FLL1_FRATIO_SHIFT);
- snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_2 + reg_offset,
- WM8994_FLL1_OUTDIV_MASK |
- WM8994_FLL1_FRATIO_MASK, reg);
-
- snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_3 + reg_offset,
- WM8994_FLL1_K_MASK, fll.k);
-
- snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_4 + reg_offset,
- WM8994_FLL1_N_MASK,
- fll.n << WM8994_FLL1_N_SHIFT);
-
- if (fll.lambda) {
- snd_soc_component_update_bits(component, WM8958_FLL1_EFS_1 + reg_offset,
- WM8958_FLL1_LAMBDA_MASK,
- fll.lambda);
- snd_soc_component_update_bits(component, WM8958_FLL1_EFS_2 + reg_offset,
- WM8958_FLL1_EFS_ENA, WM8958_FLL1_EFS_ENA);
- } else {
- snd_soc_component_update_bits(component, WM8958_FLL1_EFS_2 + reg_offset,
- WM8958_FLL1_EFS_ENA, 0);
- }
-
- snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_5 + reg_offset,
- WM8994_FLL1_FRC_NCO | WM8958_FLL1_BYP |
- WM8994_FLL1_REFCLK_DIV_MASK |
- WM8994_FLL1_REFCLK_SRC_MASK,
- ((src == WM8994_FLL_SRC_INTERNAL)
- << WM8994_FLL1_FRC_NCO_SHIFT) |
- (fll.clk_ref_div << WM8994_FLL1_REFCLK_DIV_SHIFT) |
- (src - 1));
-
- /* Clear any pending completion from a previous failure */
- try_wait_for_completion(&wm8994->fll_locked[id]);
-
- /* Enable (with fractional mode if required) */
if (freq_out) {
+ wm8994->fll_hw[id].freq_in = freq_in;
+ ret = wm_fll_set_parent(&wm8994->fll_hw[id], src);
+ if (!ret)
+ ret = wm_fll_set_rate(&wm8994->fll_hw[id], freq_out);
+ if (!ret)
+ ret = wm_fll_enable(&wm8994->fll_hw[id]);
+ if (ret < 0)
+ return ret;
+
/* Enable VMID if we need it */
if (!was_enabled) {
active_reference(component);
@@ -2273,27 +2191,6 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
break;
}
}
-
- reg = WM8994_FLL1_ENA;
-
- if (fll.k)
- reg |= WM8994_FLL1_FRAC;
- if (src == WM8994_FLL_SRC_INTERNAL)
- reg |= WM8994_FLL1_OSC_ENA;
-
- snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_1 + reg_offset,
- WM8994_FLL1_ENA | WM8994_FLL1_OSC_ENA |
- WM8994_FLL1_FRAC, reg);
-
- if (wm8994->fll_locked_irq) {
- timeout = wait_for_completion_timeout(&wm8994->fll_locked[id],
- msecs_to_jiffies(10));
- if (timeout == 0)
- dev_warn(component->dev,
- "Timed out waiting for FLL lock\n");
- } else {
- msleep(5);
- }
} else {
if (was_enabled) {
switch (control->type) {
@@ -2350,15 +2247,6 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
return 0;
}
-static irqreturn_t wm8994_fll_locked_irq(int irq, void *data)
-{
- struct completion *completion = data;
-
- complete(completion);
-
- return IRQ_HANDLED;
-}
-
static int opclk_divs[] = { 10, 20, 30, 40, 55, 60, 80, 120, 160 };
static int wm8994_set_fll(struct snd_soc_dai *dai, int id, int src,
@@ -3992,6 +3880,18 @@ static int wm8994_component_probe(struct snd_soc_component *component)
snd_soc_component_init_regmap(component, control->regmap);
+ for (i = 0; i < ARRAY_SIZE(wm8994->fll_hw); ++i) {
+ wm8994->fll_hw[i].regmap = control->regmap;
+ if (control->type == WM8994)
+ wm8994->fll_hw[i].desc = wm8994_fll_desc;
+ else
+ wm8994->fll_hw[i].desc = wm8958_fll_desc;
+
+ ret = wm_fll_init(&wm8994->fll_hw[i]);
+ if (ret)
+ return ret;
+ }
+
wm8994->hubs.component = component;
mutex_init(&wm8994->accdet_lock);
@@ -4013,9 +3913,6 @@ static int wm8994_component_probe(struct snd_soc_component *component)
INIT_DELAYED_WORK(&wm8994->mic_complete_work, wm8958_mic_work);
- for (i = 0; i < ARRAY_SIZE(wm8994->fll_locked); i++)
- init_completion(&wm8994->fll_locked[i]);
-
wm8994->micdet_irq = control->pdata.micdet_irq;
/* By default use idle_bias_off, will override for WM8994 */
@@ -4166,16 +4063,6 @@ static int wm8994_component_probe(struct snd_soc_component *component)
break;
}
- wm8994->fll_locked_irq = true;
- for (i = 0; i < ARRAY_SIZE(wm8994->fll_locked); i++) {
- ret = wm8994_request_irq(wm8994->wm8994,
- WM8994_IRQ_FLL1_LOCK + i,
- wm8994_fll_locked_irq, "FLL lock",
- &wm8994->fll_locked[i]);
- if (ret != 0)
- wm8994->fll_locked_irq = false;
- }
-
/* Make sure we can read from the GPIOs if they're inputs */
pm_runtime_get_sync(component->dev);
@@ -4377,9 +4264,6 @@ static int wm8994_component_probe(struct snd_soc_component *component)
wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_MIC1_SHRT, wm8994);
if (wm8994->micdet_irq)
free_irq(wm8994->micdet_irq, wm8994);
- for (i = 0; i < ARRAY_SIZE(wm8994->fll_locked); i++)
- wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_FLL1_LOCK + i,
- &wm8994->fll_locked[i]);
wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_DCS_DONE,
&wm8994->hubs);
wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_FIFOS_ERR, component);
@@ -4393,11 +4277,6 @@ static void wm8994_component_remove(struct snd_soc_component *component)
{
struct wm8994_priv *wm8994 = snd_soc_component_get_drvdata(component);
struct wm8994 *control = wm8994->wm8994;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(wm8994->fll_locked); i++)
- wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_FLL1_LOCK + i,
- &wm8994->fll_locked[i]);
wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_DCS_DONE,
&wm8994->hubs);
diff --git a/sound/soc/codecs/wm8994.h b/sound/soc/codecs/wm8994.h
index 1d6f2abe1c11..9c61d95c9053 100644
--- a/sound/soc/codecs/wm8994.h
+++ b/sound/soc/codecs/wm8994.h
@@ -13,6 +13,7 @@
#include <linux/mutex.h>
#include "wm_hubs.h"
+#include "wm_fll.h"
/* Sources for AIF1/2 SYSCLK - use with set_dai_sysclk() */
#define WM8994_SYSCLK_MCLK1 1
@@ -80,8 +81,7 @@ struct wm8994_priv {
int aifdiv[2];
int channels[2];
struct wm8994_fll_config fll[2], fll_suspend[2];
- struct completion fll_locked[2];
- bool fll_locked_irq;
+ struct wm_fll_data fll_hw[2];
bool fll_byp;
bool clk_has_run;
--
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
* [PATCH v2 0/4] wm8904: adapt driver for use with audio-graph-card
From: Michał Mirosław @ 2019-08-25 12:17 UTC (permalink / raw)
To: alsa-devel, patches
Cc: Kate Stewart, Maxime Jourdan, Alexandre Belloni,
Kuninori Morimoto, Kirill Marinushkin, Liam Girdwood,
Paul Cercueil, Srinivas Kandagatla, Jerome Brunet, Anders Roxell,
Takashi Iwai, Ludovic Desroches, linux-arm-kernel,
Codrin Ciubotariu, Charles Keepax, Piotr Stankiewicz,
Annaliese McDermond, Richard Fitzgerald, Mark Brown,
Nariman Poushin, Thomas Gleixner, Jaroslav Kysela, zhong jiang,
Allison Randal, Greg Kroah-Hartman, Randy Dunlap, Nikesh Oswal,
linux-kernel, Enrico Weigelt
This series allows to use WM8904 codec as audio-graph-card component.
It starts with rework of FLL handling in the codec's driver, and as an
example includes (untested) rework for codec with similar FLL: WM8994.
Series based on tiwai/sound/for-next tree. You can also pull from:
https://rere.qmqm.pl/git/linux
branch:
wm8904
(branch includes two fixes already sent to alsa-devel, but not merged yet).
Michał Mirosław (4):
ASoC: wm_fll: extract common code for Wolfson FLLs
ASoC: wm8904: use common FLL code
ASoC: wm8904: automatically choose clock source
[RFT] ASoC: wm8994: use common FLL code
sound/soc/atmel/atmel_wm8904.c | 11 +-
sound/soc/codecs/Kconfig | 9 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/wm8904.c | 516 +++++++++++---------------------
sound/soc/codecs/wm8904.h | 5 -
sound/soc/codecs/wm8994.c | 281 +++++-------------
sound/soc/codecs/wm8994.h | 4 +-
sound/soc/codecs/wm_fll.c | 518 +++++++++++++++++++++++++++++++++
sound/soc/codecs/wm_fll.h | 60 ++++
9 files changed, 849 insertions(+), 557 deletions(-)
create mode 100644 sound/soc/codecs/wm_fll.c
create mode 100644 sound/soc/codecs/wm_fll.h
--
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
* [GIT PULL] i.MX clock changes for 5.4
From: Shawn Guo @ 2019-08-25 11:55 UTC (permalink / raw)
To: Stephen Boyd
Cc: Stefan Agner, linux-imx, kernel, Fabio Estevam, linux-clk,
linux-arm-kernel
Hi Stephen,
This is the i.MX clock changes I collected for 5.4. Please help pull,
and keep commit 6ad7cb7122ce ("clk: imx8: Add DSP related clocks")
stable, as I pulled it into my DT branch as dependency. Thanks!
Shawn
The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git tags/clk-imx-5.4
for you to fetch changes up to 760e548e7f885d89bf2dfab4838df9379edd19fc:
clk: imx: imx8mn: fix audio pll setting (2019-08-24 21:04:27 +0200)
----------------------------------------------------------------
i.MX clock changes for 5.4:
- Add clock driver for i.MX8MN SoC.
- Switch i.MX8MM clock driver to platform driver.
- Add API for clk unregister when driver probe fail.
- Add Hifi4 DSP related clocks for i.MX8QXP SoC.
- Fix Audio PLL setting and parent clock for USB.
- Misc i.MX8 clock driver improvements and corrections.
----------------------------------------------------------------
Abel Vesa (3):
clk: imx: Remove unused clk based API
clk: imx8mm: Switch to platform driver
clk: imx8mq: Mark AHB clock as critical
Anson Huang (14):
dt-bindings: imx: Add clock binding doc for i.MX8MN
clk: imx8mm: Make 1416X/1443X PLL macro definitions common for usage
clk: imx: Add API for clk unregister when driver probe fail
clk: imx: Add support for i.MX8MN clock driver
clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT
clk: imx8mm: Fix typo of pwm3 clock's mux option #4
clk: imx8mm: GPT1 clock mux option #5 should be sys_pll1_80m
clk: imx7ulp: Make sure earlycon's clock is enabled
clk: imx: Remove unused function statement
clk: imx8mn: Keep uart clocks on for early console
clk: imx8mm: Unregister clks when of_clk_add_provider failed
clk: imx8mq: Unregister clks when of_clk_add_provider failed
clk: imx8mn: Add missing rate_count assignment for each PLL structure
clk: imx8mn: Add necessary frequency support for ARM PLL table
Daniel Baluta (1):
clk: imx8: Add DSP related clocks
Fancy Fang (1):
clk: imx8mm: rename 'share_count_dcss' to 'share_count_disp'
Leonard Crestez (4):
clk: imx8mq: Fix sys3 pll references
clk: imx8mm: Fix incorrect parents
clk: imx8mn: Fix incorrect parents
clk: imx8mn: Add GIC clock
Li Jun (2):
clk: imx8mm: correct the usb1_ctrl parent to be usb_bus
clk: imx8mq: set correct parent for usb ctrl clocks
Peng Fan (3):
clk: imx: imx8mm: fix audio pll setting
clk: imx8mn: fix int pll clk gate
clk: imx: imx8mn: fix audio pll setting
.../devicetree/bindings/clock/imx8mn-clock.yaml | 112 ++++
drivers/clk/imx/Kconfig | 6 +
drivers/clk/imx/Makefile | 1 +
drivers/clk/imx/clk-imx7ulp.c | 31 +
drivers/clk/imx/clk-imx8mm.c | 109 ++--
drivers/clk/imx/clk-imx8mn.c | 660 +++++++++++++++++++++
drivers/clk/imx/clk-imx8mq.c | 131 ++--
drivers/clk/imx/clk-imx8qxp-lpcg.c | 5 +
drivers/clk/imx/clk.c | 8 +
drivers/clk/imx/clk.h | 43 +-
include/dt-bindings/clock/imx8-clock.h | 6 +-
include/dt-bindings/clock/imx8mn-clock.h | 216 +++++++
12 files changed, 1188 insertions(+), 140 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/imx8mn-clock.yaml
create mode 100644 drivers/clk/imx/clk-imx8mn.c
create mode 100644 include/dt-bindings/clock/imx8mn-clock.h
_______________________________________________
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 net-next v3 2/3] net: ethernet: mediatek: Re-add support SGMII
From: René van Dorst @ 2019-08-25 11:41 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Nelson Chang, Frank Wunderlich, netdev, Sean Wang, linux-mips,
linux-mediatek, John Crispin, Matthias Brugger, Stefan Roese,
David S . Miller, linux-arm-kernel
In-Reply-To: <20190824133225.GE13294@shell.armlinux.org.uk>
Hi Russell,
Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>:
> Hi René,
>
> On Sat, Aug 24, 2019 at 01:11:17PM +0000, René van Dorst wrote:
>> Hi Russell,
>>
>> Mediatek calls it Turbo RGMII. It is a overclock version of RGMII mode.
>> It is used between first GMAC and port 6 of the mt7530 switch. Can be used
>> with
>> an internal and an external mt7530 switch.
>>
>> TRGMII speed are:
>> * mt7621: 1200Mbit
>> * mt7623: 2000Mbit and 2600Mbit.
>>
>> I think that TRGMII is only used in a fixed-link situation in combination
>> with a
>> mt7530 switch and running and maximum speed/full duplex. So reporting
>> 1000baseT_Full seems to me the right option.
>
> I think we can ignore this one for the purposes of merging this patch
> set, since this seems to be specific to this setup. Neither 1000BaseT
> nor 1000BaseX fit very well, but we have to choose something.
>
>> PHY_INTERFACE_MODE_GMII:
>> 10baseT_Half
>> 10baseT_Full
>> 100baseT_Half
>> 100baseT_Full
>> 1000baseT_Half
>> 1000baseT_Full
>
> I think GMII can be connected to a PHY that can convert to 1000BaseX, so
> should probably include that here too.
>
Thanks for reviewing.
I shall add that too.
I send v4 today.
Greats,
René
> Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Jerry-ch Chen @ 2019-08-25 9:18 UTC (permalink / raw)
To: Tomasz Figa
Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
Frederic Chen (陳俊元),
Rynn Wu (吳育恩), srv_heupstream, po-yang.huang,
suleiman@chromium.org, shik@chromium.org, jerry-ch.chen,
Jungo Lin (林明俊),
Sj Huang (黃信璋), yuzhao@chromium.org,
linux-mediatek@lists.infradead.org, zwisler@chromium.org,
hans.verkuil@cisco.com, matthias.bgg@gmail.com,
Christie Yu (游雅惠), mchehab@kernel.org,
laurent.pinchart+renesas@ideasonboard.com,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <20190802082815.GA203993@chromium.org>
Hi Tomasz,
On Fri, 2019-08-02 at 16:28 +0800, Tomasz Figa wrote:
> Hi Jerry,
>
> On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
> > From: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
> >
> > This patch adds the driver of Face Detection (FD) unit in
> > Mediatek camera system, providing face detection function.
> >
> > The mtk-isp directory will contain drivers for multiple IP
> > blocks found in Mediatek ISP system. It will include ISP Pass 1
> > driver (CAM), sensor interface driver, DIP driver and face
> > detection driver.
> >
> > Signed-off-by: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
> > ---
> > drivers/media/platform/Makefile | 2 +
> > drivers/media/platform/mtk-isp/fd/Makefile | 5 +
> > drivers/media/platform/mtk-isp/fd/mtk_fd.h | 157 +++
> > drivers/media/platform/mtk-isp/fd/mtk_fd_40.c | 1259 +++++++++++++++++++++++++
> > include/uapi/linux/v4l2-controls.h | 4 +
> > 5 files changed, 1427 insertions(+)
> > create mode 100644 drivers/media/platform/mtk-isp/fd/Makefile
> > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
> >
>
> Thanks for the patch! I finally got a chance to fully review the code. Sorry
> for the delay. Please check my comments inline.
>
I appreciate your comments.
I've fixed most of the comments and verifying them,
Sorry for the delay, here is the reply.
>
> > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> > index e6deb25..8b817cc 100644
> > --- a/drivers/media/platform/Makefile
> > +++ b/drivers/media/platform/Makefile
> > @@ -94,6 +94,8 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP) += mtk-mdp/
> >
> > obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk-jpeg/
> >
> > +obj-$(CONFIG_VIDEO_MEDIATEK_FD) += mtk-isp/fd/
> > +
> > obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom/camss/
> >
> > obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
> > diff --git a/drivers/media/platform/mtk-isp/fd/Makefile b/drivers/media/platform/mtk-isp/fd/Makefile
> > new file mode 100644
> > index 0000000..9b1c501
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-isp/fd/Makefile
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +mtk-fd-objs += mtk_fd_40.o
> > +
> > +obj-$(CONFIG_VIDEO_MEDIATEK_FD) += mtk-fd.o
> > \ No newline at end of file
> > diff --git a/drivers/media/platform/mtk-isp/fd/mtk_fd.h b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > new file mode 100644
> > index 0000000..289999b
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > @@ -0,0 +1,157 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +//
> > +// Copyright (c) 2018 MediaTek Inc.
> > +
> > +#ifndef __MTK_FD_HW_H__
> > +#define __MTK_FD_HW_H__
> > +
> > +#include <linux/io.h>
> > +#include <linux/types.h>
> > +#include <linux/platform_device.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/videobuf2-v4l2.h>
> > +
> > +#define MTK_FD_OUTPUT_MIN_WIDTH 26U
> > +#define MTK_FD_OUTPUT_MIN_HEIGHT 26U
> > +#define MTK_FD_OUTPUT_MAX_WIDTH 640U
> > +#define MTK_FD_OUTPUT_MAX_HEIGHT 480U
> > +
> > +/* Control the user defined image widths and heights
> > + * to be scaled and performed face detection in FD HW.
> > + * MTK FD support up to 14 user defined image sizes to perform face detection.
> > + */
> > +#define V4L2_CID_MTK_FD_SCALE_IMG_WIDTH (V4L2_CID_USER_MTK_FD_BASE + 1)
> > +#define V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT (V4L2_CID_USER_MTK_FD_BASE + 2)
> > +
> > +/* Control the numbers of user defined image sizes.
> > + * The default value is 0 which means user is not going
> > + * to define the specific image sizes.
> > + */
> > +#define V4L2_CID_MTK_FD_SCALE_IMG_NUM (V4L2_CID_USER_MTK_FD_BASE + 3)
> > +
> > +/* Control the Face Pose to be detected.
> > + * Here describe the value as following:
> > + * {0, detect the front face with rotation from 0 to 270 degrees},
> > + * {1, detect the front face with rotation from 0 to 240 and 300 degrees},
> > + * {2, detect the front face with rotation from 0 to 240 and 330 degrees},
> > + * {3, detect the front face with rotation from 0 to 240 and left side face}.
> > + */
> > +#define V4L2_CID_MTK_FD_DETECT_POSE (V4L2_CID_USER_MTK_FD_BASE + 4)
> > +#define V4L2_CID_MTK_FD_DETECT_SPEEDUP (V4L2_CID_USER_MTK_FD_BASE + 5)
> > +#define V4L2_CID_MTK_FD_EXTRA_MODEL (V4L2_CID_USER_MTK_FD_BASE + 6)
> > +
> > +/* We reserve 16 controls for this driver. */
> > +#define V4L2_CID_MTK_FD_MAX 16
> > +
> > +#define ENABLE_FD 0x111
> > +#define FD_HW_ENABLE 0x4
> > +#define FD_INT_EN 0x15c
> > +#define FD_INT 0x168
> > +#define FD_RESULT 0x178
> > +#define FD_IRQ_MASK 0x001
> > +
> > +#define RS_MAX_BUF_SIZE 2288788
> > +#define FD_MAX_SPEEDUP 7
> > +#define FD_MAX_POSE_VAL 0xfffffffffffffff
> > +#define FD_DEF_POSE_VAL 0x3ff
> > +#define MAX_FD_SEL_NUM 1026
> > +
> > +/* The max. number of face sizes could be detected, for feature scaling */
> > +#define FACE_SIZE_NUM_MAX 14
> > +/* FACE_SIZE_NUM_MAX + 1, first scale for input image W/H */
> > +#define FD_SCALE_NUM 15
> > +
> > +enum fd_state {
> > + FD_ENQ,
> > + FD_CBD,
> > +};
> > +
> > +enum fd_img_format {
> > + FMT_VYUY = 2,
> > + FMT_UYVY,
> > + FMT_YVYU,
> > + FMT_YUYV,
> > + FMT_UNKNOWN
> > +};
>
> Please use #define macros for hardware/firmware interface definitions.
>
Ok, I will refine as following
#define MTK_FD_HW_FMT_VYUY 2
#define MTK_FD_HW_FMT_UYVY 3
#define MTK_FD_HW_FMT_YVYU 4
#define MTK_FD_HW_FMT_YUYV 5
#define MTK_FD_HW_FMT_UNKNOWN 6
> > +
> > +struct fd_buffer {
> > + __u32 scp_addr; /* used by SCP */
> > + __u32 dma_addr; /* used by DMA HW */
> > +} __packed;
> > +
> > +enum fd_scp_cmd {
> > + FD_CMD_INIT,
> > + FD_CMD_ENQUEUE,
> > +};
>
> Ditto.
Ok, I will refine as following
#define MTK_FD_IPI_CMD_INIT 0
#define MTK_FD_IPI_CMD_ENQUEUE 1
>
> > +
> > +struct fd_user_output {
> > + __u64 results[MAX_FD_SEL_NUM];
> > + __u16 number;
> > +};
> > +
> > +struct user_param {
> > + u8 fd_pose;
> > + u8 fd_speedup;
> > + u8 fd_extra_model;
> > + u8 scale_img_num;
> > + u8 src_img_fmt;
> > + __u16 scale_img_width[FD_SCALE_NUM];
> > + __u16 scale_img_height[FD_SCALE_NUM];
> > +} __packed;
> > +
> > +struct fd_hw_param {
> > + struct fd_buffer src_img;
> > + struct fd_buffer user_result;
> > + struct user_param user_param;
> > +} __packed;
> > +
> > +struct cmd_init_info {
> > + struct fd_buffer fd_manager;
> > + __u32 rs_dma_addr;
> > +} __packed;
> > +
> > +struct ipi_message {
> > + u8 cmd_id;
> > + union {
> > + struct cmd_init_info init_param;
> > + struct fd_hw_param hw_param;
> > + };
> > +} __packed;
> > +
> > +struct mtk_fd_hw {
> > + struct clk *fd_clk;
> > + struct rproc *rproc_handle;
> > + struct platform_device *scp_pdev;
> > + struct fd_buffer scp_mem;
> > + wait_queue_head_t wq;
> > + void __iomem *fd_base;
> > + atomic_t fd_user_cnt;
> > + enum fd_state state;
> > + u32 fd_irq_result;
> > + /* Ensure only one job in hw */
> > + struct mutex fd_hw_lock;
> > +};
> > +
> > +struct mtk_fd_dev {
> > + struct platform_device *pdev;
> > + struct v4l2_device v4l2_dev;
> > + struct v4l2_m2m_dev *m2m_dev;
> > + struct media_device mdev;
> > + struct video_device vfd;
> > + struct mtk_fd_hw fd_hw;
>
> Could we just put the contents of that struct directly here? That should
> simplify dereference chains in the code.
>
Ok, I will fix it.
> > + /* Lock for V4L2 operations */
> > + struct mutex vfd_lock;
> > +};
> > +
> > +struct mtk_fd_ctx {
> > + struct mtk_fd_dev *fd_dev;
> > + struct device *dev;
> > + struct v4l2_fh fh;
> > + struct v4l2_ctrl_handler hdl;
> > + struct v4l2_pix_format_mplane src_fmt;
> > + struct v4l2_meta_format dst_fmt;
> > + struct user_param user_param;
> > +};
> > +
> > +#endif/*__MTK_FD_HW_H__*/
> > diff --git a/drivers/media/platform/mtk-isp/fd/mtk_fd_40.c b/drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
> > new file mode 100644
> > index 0000000..246d3aa
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
> > @@ -0,0 +1,1259 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Copyright (c) 2018 MediaTek Inc.
> > +
> > +#include <linux/clk.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_data/mtk_scp.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/remoteproc.h>
> > +#include <media/videobuf2-dma-contig.h>
> > +#include <media/videobuf2-v4l2.h>
> > +#include <linux/wait.h>
> > +#include <media/v4l2-ioctl.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-mem2mem.h>
> > +#include <media/videobuf2-core.h>
> > +
> > +#include "mtk_fd.h"
> > +
> > +static struct v4l2_meta_format fw_param_fmts[] = {
>
> const?
>
> Also, isn't this the buffer with the detected faces rather than params?
This struct will be removed. Yes, it's the buffer with detected faces.
>
> > + {
> > + .dataformat = V4L2_META_FMT_MTISP_PARAMS,
>
> This should use its own format.
Ok, I will define V4L2_META_FMT_MTFD_RESULT in videodev2.h
>
> > + .buffersize = 1024 * 30,
>
> Please define a C struct for this buffer and use sizeof() here.
>
Ok, that will be sizeof(struct fd_user_output);
> > + },
> > +};
>
> Actually, is there a reason to have this array at all if there is only 1
> format supported? I think we could just put the right constants directly in
> the code.
>
I agree, I'll remove this array and use the constants in the code.
> > +
> > +static const struct v4l2_pix_format_mplane in_img_fmts[] = {
> > + {
> > + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> > + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> > + .pixelformat = V4L2_PIX_FMT_VYUY,
> > + .colorspace = V4L2_COLORSPACE_BT2020,
> > + .field = V4L2_FIELD_NONE,
> > + .num_planes = 1,
> > + },
> > + {
> > + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> > + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> > + .pixelformat = V4L2_PIX_FMT_YUYV,
> > + .colorspace = V4L2_COLORSPACE_BT2020,
> > + .field = V4L2_FIELD_NONE,
> > + .num_planes = 1,
> > + },
> > + {
> > + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> > + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> > + .pixelformat = V4L2_PIX_FMT_YVYU,
> > + .colorspace = V4L2_COLORSPACE_BT2020,
> > + .field = V4L2_FIELD_NONE,
> > + .num_planes = 1,
> > + },
> > + {
> > + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> > + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> > + .pixelformat = V4L2_PIX_FMT_UYVY,
> > + .colorspace = V4L2_COLORSPACE_BT2020,
> > + .field = V4L2_FIELD_NONE,
> > + .num_planes = 1,
>
> If all the formats have the same values for a field, there is no reason to
> have the field initialized here. Just make initialize them to the constant
> values inside TRY_FMT.
>
Ok, I will do so.
> Hmm, are the interleaved YUV formats the only formats supported by this
> hardware? That would be quite unfortunate given the formats supported by the
> other IP blocks on this SoC use the more typical planar formats.
>
The supported YUV 1 plane formats are listed above.
The rest supported format is YUV 2 plane format, which should be
V4L2_PIX_FMT_NV16M.
For the in_img_fmts[], I will refine as following:
static const struct v4l2_pix_format_mplane mtk_fd_img_fmts[] = {
{
.pixelformat = V4L2_PIX_FMT_VYUY,
.num_planes = 1,
},
{
.pixelformat = V4L2_PIX_FMT_YUYV,
.num_planes = 1,
},
{
.pixelformat = V4L2_PIX_FMT_YVYU,
.num_planes = 1,
},
{
.pixelformat = V4L2_PIX_FMT_UYVY,
.num_planes = 1,
},
{
.pixelformat = V4L2_PIX_FMT_NV16M,
.num_planes = 2,
},
{
.pixelformat = V4L2_PIX_FMT_NV61M,
.num_planes = 2,
},
};
> > + },
> > +};
> > +
> > +#define NUM_FORMATS ARRAY_SIZE(in_img_fmts)
> > +
> > +static inline struct mtk_fd_dev *mtk_fd_hw_to_dev(struct mtk_fd_hw *fd_hw)
> > +{
> > + return container_of(fd_hw, struct mtk_fd_dev, fd_hw);
> > +}
> > +
> > +static inline struct mtk_fd_ctx *fh_to_ctx(struct v4l2_fh *fh)
> > +{
> > + return container_of(fh, struct mtk_fd_ctx, fh);
> > +}
> > +
> > +static int mtk_fd_load_scp(struct mtk_fd_hw *fd_hw)
> > +{
> > + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> > + struct device *dev = &fd_dev->pdev->dev;
> > + phandle rproc_phandle;
> > + int ret;
> > +
> > + /* init scp */
> > + fd_hw->scp_pdev = scp_get_pdev(fd_dev->pdev);
> > + if (!fd_hw->scp_pdev) {
> > + dev_err(dev, "Failed to get scp device\n");
> > + return -ENODEV;
> > + }
> > +
> > + if (of_property_read_u32(fd_dev->pdev->dev.of_node, "mediatek,scp",
> > + &rproc_phandle)) {
> > + dev_err(dev, "Could not get scp device\n");
> > + return -EINVAL;
> > + }
> > +
> > + fd_hw->rproc_handle = rproc_get_by_phandle(rproc_phandle);
> > + if (!fd_hw->rproc_handle) {
> > + dev_err(dev, "Could not get FD's rproc_handle\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = rproc_boot(fd_hw->rproc_handle);
> > + if (ret < 0) {
> > + /**
> > + * Return 0 if downloading firmware successfully,
> > + * otherwise it is failed
> > + */
> > + dev_err(dev, "rproc_boot failed\n");
> > + return -ENODEV;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static dma_addr_t mtk_fd_hw_alloc_rs_dma_addr(struct mtk_fd_hw *fd_hw)
> > +{
> > + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> > + struct device *dev = &fd_dev->pdev->dev;
> > + void *va;
> > + dma_addr_t dma_handle;
> > +
> > + va = dma_alloc_coherent(dev, RS_MAX_BUF_SIZE, &dma_handle, GFP_KERNEL);
>
> I see this allocated here, but I don't see this freed anywhere.
>
I will fix it in next version.
> > + if (!va) {
> > + dev_err(dev, "dma_alloc null va\n");
> > + return -ENOMEM;
> > + }
> > + memset(va, 0, RS_MAX_BUF_SIZE);
> > +
> > + return dma_handle;
> > +}
> > +
> > +static int mtk_fd_send_ipi_init(struct mtk_fd_hw *fd_hw)
> > +{
> > + struct ipi_message fd_init_msg;
> > + dma_addr_t rs_dma_addr;
> > +
> > + fd_init_msg.cmd_id = FD_CMD_INIT;
> > +
> > + fd_init_msg.init_param.fd_manager.scp_addr = fd_hw->scp_mem.scp_addr;
> > + fd_init_msg.init_param.fd_manager.dma_addr = fd_hw->scp_mem.dma_addr;
> > +
> > + rs_dma_addr = mtk_fd_hw_alloc_rs_dma_addr(fd_hw);
> > + if (!rs_dma_addr)
> > + return -ENOMEM;
> > +
> > + fd_init_msg.init_param.rs_dma_addr = rs_dma_addr;
> > +
> > + return scp_ipi_send(fd_hw->scp_pdev, SCP_IPI_FD_CMD, &fd_init_msg,
> > + sizeof(fd_init_msg), 0);
> > +}
> > +
> > +static int mtk_fd_hw_enable(struct mtk_fd_hw *fd_hw)
> > +{
> > + int ret;
> > +
> > + ret = mtk_fd_load_scp(fd_hw);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mtk_fd_send_ipi_init(fd_hw);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_hw_connect(struct mtk_fd_hw *fd_hw)
> > +{
> > + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> > + s32 usercount;
> > +
> > + mutex_lock(&fd_hw->fd_hw_lock);
>
> First of all, we don't need a mutex here, because all the V4L2 ioctls are
> already running with the video device mutex.
Ok, I will remove the using of the mutex here.
>
> > + usercount = atomic_inc_return(&fd_hw->fd_user_cnt);
>
> If we already use a mutex, there is no point in using atomic types.
>
Ok, I will replace the atomic types with normal types
> > + if (usercount == 1) {
> > + pm_runtime_get_sync(&fd_dev->pdev->dev);
> > + if (mtk_fd_hw_enable(fd_hw)) {
> > + pm_runtime_put_sync(&fd_dev->pdev->dev);
> > + atomic_dec_return(&fd_hw->fd_user_cnt);
> > + mutex_unlock(&fd_hw->fd_hw_lock);
> > + return -EINVAL;
> > + }
> > + }
>
> This is a simple mem-to-mem device, so there is no reason to keep it active
> all the time it's streaming. Please just get the runtime PM counter when
> queuing a job to the hardware and release it when the job finishes.
>
> I guess we might still want to do the costly operations like rproc_boot()
> when we start streaming, though.
>
Do you mean by moving the pm_runtime_get/put stuff to the job execution
and job finish place?
the rproc_boot() operation will be done here.
> > + mutex_unlock(&fd_hw->fd_hw_lock);
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_wait_irq(struct mtk_fd_hw *fd_hw)
> > +{
> > + int timeout;
> > + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> > + struct device *dev = &fd_dev->pdev->dev;
> > +
> > + timeout = wait_event_interruptible_timeout
> > + (fd_hw->wq, (fd_hw->fd_irq_result & FD_IRQ_MASK),
> > + usecs_to_jiffies(1000000));
> > + if (!timeout) {
> > + dev_err(dev, "%s timeout, %d\n", __func__,
> > + fd_hw->fd_irq_result);
> > + return -EAGAIN;
> > + } else if (!(fd_hw->fd_irq_result & FD_IRQ_MASK)) {
> > + dev_err(dev, "%s No IRQ mask:0x%8x\n",
> > + __func__, fd_hw->fd_irq_result);
> > + return -EINVAL;
> > + }
> > + fd_hw->fd_irq_result = 0;
> > +
> > + return 0;
> > +}
> > +
> > +static void mtk_fd_hw_disconnect(struct mtk_fd_hw *fd_hw)
> > +{
> > + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> > + s32 usercount;
> > +
> > + mutex_lock(&fd_hw->fd_hw_lock);
> > + atomic_dec_return(&fd_hw->fd_user_cnt);
> > + usercount = atomic_read(&fd_hw->fd_user_cnt);
> > + if (usercount == 0) {
> > + if (fd_hw->state == FD_ENQ)
> > + mtk_fd_wait_irq(fd_hw);
>
> This shouldn't be possible to happen as the queues should be already stopped
> at this point.
>
Ok, I will remove it.
> > +
> > + pm_runtime_put_sync(&fd_dev->pdev->dev);
>
> Any reason to use pm_runtime_put_sync() over pm_runtime_put()?
>
No special reason to do so, the pm_runtime_put_sync here will be moved
to the place each job finished.
> > + rproc_shutdown(fd_hw->rproc_handle);
> > + rproc_put(fd_hw->rproc_handle);
> > + }
> > + mutex_unlock(&fd_hw->fd_hw_lock);
> > +}
> > +
> > +static void mtk_fd_hw_job_finish(struct mtk_fd_hw *fd_hw,
> > + struct fd_hw_param *fd_param,
> > + enum vb2_buffer_state vb_state)
> > +{
> > + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> > + struct mtk_fd_ctx *ctx;
> > + struct device *dev = &fd_dev->pdev->dev;
> > + struct vb2_buffer *src_vb, *dst_vb;
> > + struct vb2_v4l2_buffer *src_vbuf = NULL, *dst_vbuf = NULL;
> > +
> > + ctx = v4l2_m2m_get_curr_priv(fd_dev->m2m_dev);
> > + if (!ctx) {
> > + dev_err(dev, "Instance released before end of transaction\n");
> > + return;
> > + }
>
> This shouldn't be possible. I suspect you're seeing this because
> .stop_streaming is not implemented correctly. See my comment there.
>
Ok, I will remove this.
> > +
> > + src_vb = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > + if (WARN_ON(!src_vb))
> > + return;
>
> This shouldn't be possible.
>
Ditto.
> > + src_vbuf = to_vb2_v4l2_buffer(src_vb);
> > +
> > + dst_vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > + if (WARN_ON(!dst_vb))
> > + return;
>
> Ditto.
>
Ditto.
> > + dst_vbuf = to_vb2_v4l2_buffer(dst_vb);
> > +
> > + dst_vbuf->vb2_buf.timestamp = src_vbuf->vb2_buf.timestamp;
> > + dst_vbuf->timecode = src_vbuf->timecode;
> > + dst_vbuf->flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
> > + dst_vbuf->flags |= src_vbuf->flags & V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
>
> We should be able to just use v4l2_m2m_buf_copy_metadata().
>
Ok, I will refine as:
v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf,
V4L2_BUF_FLAG_TSTAMP_SRC_MASK);
> > +
> > + v4l2_m2m_buf_done(src_vbuf, vb_state);
> > + v4l2_m2m_buf_done(dst_vbuf, vb_state);
> > + v4l2_m2m_job_finish(fd_dev->m2m_dev, ctx->fh.m2m_ctx);
> > +}
> > +
> > +static int mtk_fd_hw_job_exec(struct mtk_fd_hw *fd_hw,
> > + struct fd_hw_param *fd_param,
> > + void *output_vaddr)
> > +{
> > + struct fd_user_output *fd_output;
> > + struct ipi_message fd_ipi_msg;
> > + int ret;
> > + u32 num;
> > +
> > + if (fd_param->user_param.src_img_fmt == FMT_UNKNOWN)
> > + goto param_err;
>
> Is this possible?
>
Only if user set wrong format, I will remove this.
> > +
> > + mutex_lock(&fd_hw->fd_hw_lock);
> > + fd_hw->state = FD_ENQ;
>
> What is this state for?
>
It was for checking status, if a job is processing, the state is
FD_ENQ,
then we should wait for the last job to be done when pm_suspend().
> > + fd_ipi_msg.cmd_id = FD_CMD_ENQUEUE;
> > + memcpy(&fd_ipi_msg.hw_param, fd_param, sizeof(fd_ipi_msg.hw_param));
> > + ret = scp_ipi_send(fd_hw->scp_pdev, SCP_IPI_FD_CMD, &fd_ipi_msg,
> > + sizeof(fd_ipi_msg), 0);
> > + if (ret)
> > + goto buf_err;
> > +
> > + ret = mtk_fd_wait_irq(fd_hw);
> > + if (ret)
> > + goto buf_err;
>
> This function is called from device_run and it shouldn't be synchronous. It
> should only queue the job to the hardware to be handled asynchronously when
> it finishes.
>
Ok, I will fix it.
> > +
> > + num = readl(fd_hw->fd_base + FD_RESULT);
> > + /* Disable FD ISR */
> > + writel(0x0, fd_hw->fd_base + FD_INT_EN);
> > +
> > + fd_output = (struct fd_user_output *)output_vaddr;
> > + fd_output->number = num;
> > + fd_hw->state = FD_CBD;
> > + mutex_unlock(&fd_hw->fd_hw_lock);
> > +
> > + mtk_fd_hw_job_finish(fd_hw, fd_param, VB2_BUF_STATE_DONE);
> > + return 0;
> > +
> > +buf_err:
> > + mutex_unlock(&fd_hw->fd_hw_lock);
> > +param_err:
> > + mtk_fd_hw_job_finish(fd_hw, fd_param, VB2_BUF_STATE_ERROR);
> > + return ret;
> > +}
> > +
> > +static int mtk_fd_vb2_buf_out_validate(struct vb2_buffer *vb)
> > +{
> > + struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb);
> > +
> > + v4l2_buf->field = V4L2_FIELD_NONE;
>
> We need to fail with -EINVAL if v4l2_buf->field was different than
> V4L2_FIELD_ANY or _NONE.
>
Ok, I will refine as following:
if (v4l2_buf->field == V4L2_FIELD_ANY)
v4l2_buf->field = V4L2_FIELD_NONE;
if (v4l2_buf->field != V4L2_FIELD_NONE)
return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_vb2_buf_prepare(struct vb2_buffer *vb)
> > +{
> > + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > + struct vb2_queue *vq = vb->vb2_queue;
> > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > + struct device *dev = ctx->dev;
> > + struct v4l2_pix_format_mplane *pixfmt;
> > +
> > + switch (vq->type) {
> > + case V4L2_BUF_TYPE_META_CAPTURE:
> > + if (vb2_plane_size(vb, 0) < ctx->dst_fmt.buffersize) {
> > + dev_err(dev, "meta size %d is too small\n");
>
> Please make this dev_dbg(), because userspace misbehavior must not trigger
> kernel error logs.
>
Ok, fixed.
> > + return -EINVAL;
> > + }
> > + break;
> > + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > + pixfmt = &ctx->src_fmt;
> > +
> > + if (vbuf->field == V4L2_FIELD_ANY)
> > + vbuf->field = V4L2_FIELD_NONE;
> > +
> > + if (vb->num_planes != 1 || vbuf->field != V4L2_FIELD_NONE) {
> > + dev_err(dev, "plane or field %d not supported\n",
> > + vb->num_planes, vbuf->field);
>
> Ditto.
>
Done.
> > + return -EINVAL;
> > + }
> > + if (vb2_plane_size(vb, 0) < pixfmt->plane_fmt[0].sizeimage) {
> > + dev_err(dev, "plane %d is too small\n");
>
> Ditto.
>
Done.
> > + return -EINVAL;
> > + }
> > + break;
> > + default:
> > + dev_err(dev, "invalid queue type: %d\n", vq->type);
> > + return -EINVAL;
>
> We shouldn't need to handle this.
>
Ok, I will remove this.
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void mtk_fd_vb2_buf_queue(struct vb2_buffer *vb)
> > +{
> > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > +
> > + v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
> > +}
> > +
> > +static int mtk_fd_vb2_queue_setup(struct vb2_queue *vq,
> > + unsigned int *num_buffers,
> > + unsigned int *num_planes,
> > + unsigned int sizes[],
> > + struct device *alloc_devs[])
> > +{
> > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > + struct device *dev = ctx->dev;
> > + unsigned int size;
> > +
> > + switch (vq->type) {
> > + case V4L2_BUF_TYPE_META_CAPTURE:
> > + size = ctx->dst_fmt.buffersize;
> > + break;
> > + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > + size = ctx->src_fmt.plane_fmt[0].sizeimage;
> > + break;
> > + default:
> > + dev_err(dev, "invalid queue type: %d\n", vq->type);
>
> We should need to handle this.
>
Do you mean by giving a size instead of return -EINVAL?
> > + return -EINVAL;
> > + }
> > +
> > + if (!*num_planes) {
> > + *num_planes = 1;
> > + sizes[0] = size;
> > + }
>
> We need to handle the case when *num_planes is non-zero. We then need to
> check if it's 1 and *sizes >= size and fail with -EINVAL if not.
>
Ok, I will refine as following:
if (*num_planes) {
if (sizes[0] < size || *num_planes != 1)
return -EINVAL;
} else {
*num_planes = 1;
sizes[0] = size;
}
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_vb2_start_streaming(struct vb2_queue *vq, unsigned int count)
> > +{
> > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > +
> > + return mtk_fd_hw_connect(&ctx->fd_dev->fd_hw);
>
> This would be called twice for every context, once for the VIDEO_OUTPUT
> queue and once for the META_CAPTURE queue. Perhaps it would make sense to
> just do this for the VIDEO_OUTPUT queue?
>
Ok, I will refine as:
if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
return mtk_fd_hw_connect(&ctx->fd_dev->fd_hw);
else
return 0;
> > +}
> > +
> > +static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > +{
> > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > + struct vb2_buffer *vb;
>
> How do we guarantee here that the hardware isn't still accessing the buffers
> removed below?
>
Maybe we can check the driver state flag and aborting the unfinished
jobs?
(fd_hw->state == FD_ENQ)
> > +
> > + if (V4L2_TYPE_IS_OUTPUT(vq->type))
> > + vb = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > + else
> > + vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > +
> > + while (vb) {
> > + v4l2_m2m_buf_done(to_vb2_v4l2_buffer(vb), VB2_BUF_STATE_ERROR);
> > + if (V4L2_TYPE_IS_OUTPUT(vq->type))
> > + vb = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > + else
> > + vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > + }
>
> We can use v4l2_m2m_buf_remove(). Also we can move the call into the loop
> condition:
>
> while ((vb == v4l2_m2m_buf_remove(...)))
> v4l2_m2m_buf_done(...);
>
Ok, I will refine as following:
while ((vb = v4l2_m2m_buf_remove(V4L2_TYPE_IS_OUTPUT(vq->type)?
&m2m_ctx->out_q_ctx :
&m2m_ctx->cap_q_ctx)))
v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > +
> > + mtk_fd_hw_disconnect(&ctx->fd_dev->fd_hw);
>
> This should also probably be done only for 1 queue. Since the VIDEO_OUTPUT
> queue supports requestes, I'd consider it the master one.
>
Ok, only for output queue to trigger disconnect.
if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
mtk_fd_hw_disconnect(&ctx->fd_dev->fd_hw);
> > +}
> > +
> > +static void mtk_fd_vb2_request_complete(struct vb2_buffer *vb)
> > +{
> > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > +
> > + v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->hdl);
> > +}
> > +
> > +static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
> > + const struct v4l2_pix_format_mplane *sfmt)
> > +{
> > + dfmt->width = sfmt->width;
> > + dfmt->height = sfmt->height;
> > + dfmt->pixelformat = sfmt->pixelformat;
> > + dfmt->field = sfmt->field;
> > + dfmt->colorspace = sfmt->colorspace;
> > + dfmt->num_planes = sfmt->num_planes;
> > +
> > + /* Use default */
> > + dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > + dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> > + dfmt->xfer_func =
> > + V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> > + dfmt->plane_fmt[0].bytesperline = dfmt->width * 2;
> > + dfmt->plane_fmt[0].sizeimage =
> > + dfmt->height * dfmt->plane_fmt[0].bytesperline;
> > + memset(dfmt->reserved, 0, sizeof(dfmt->reserved));
> > +}
>
> Could we unify this function with mtk_fd_m2m_try_fmt_out_mp()? That function
> should be almost directly reusable for the default format initialization +/-
> the arguments passed to it.
>
Ok, I will try to reuse it as following:
static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
const struct v4l2_pix_format_mplane *sfmt)
{
dfmt->field = V4L2_FIELD_NONE;
dfmt->colorspace = V4L2_COLORSPACE_BT2020;
dfmt->num_planes = sfmt->num_planes;
dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
dfmt->xfer_func =
V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
/* Keep user setting as possible */
dfmt->width = clamp(dfmt->width,
MTK_FD_OUTPUT_MIN_WIDTH,
MTK_FD_OUTPUT_MAX_WIDTH);
dfmt->height = clamp(dfmt->height,
MTK_FD_OUTPUT_MIN_HEIGHT,
MTK_FD_OUTPUT_MAX_HEIGHT);
if (sfmt->num_planes == 2) {
/* NV16M and NV61M has 1 byte per pixel */
dfmt->plane_fmt[0].bytesperline = dfmt->width;
dfmt->plane_fmt[1].bytesperline = dfmt->width;
} else {
/* 2 bytes per pixel */
dfmt->plane_fmt[0].bytesperline = dfmt->width * 2;
}
dfmt->plane_fmt[0].sizeimage =
dfmt->height * dfmt->plane_fmt[0].bytesperline;
}
> > +
> > +static const struct v4l2_pix_format_mplane *mtk_fd_find_fmt(u32 format)
> > +{
> > + unsigned int i;
> > + const struct v4l2_pix_format_mplane *dev_fmt;
> > +
> > + for (i = 0; i < NUM_FORMATS; i++) {
> > + dev_fmt = &in_img_fmts[i];
> > + if (dev_fmt->pixelformat == format)
> > + return dev_fmt;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static int mtk_fd_m2m_querycap(struct file *file, void *fh,
> > + struct v4l2_capability *cap)
> > +{
> > + struct mtk_fd_dev *fd_dev = video_drvdata(file);
> > + struct device *dev = &fd_dev->pdev->dev;
> > +
> > + strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
>
> Please use dev_driver_string().
>
Done,
> > + strscpy(cap->card, fd_dev->vfd.name, sizeof(cap->card));
>
> I think we can also make this dev_driver_string().
>
Done.
> > + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > + dev_name(&fd_dev->pdev->dev));
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_m2m_enum_fmt_out_mp(struct file *file, void *fh,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < NUM_FORMATS; ++i) {
> > + if (i == f->index) {
> > + f->pixelformat = in_img_fmts[i].pixelformat;
> > + return 0;
> > + }
> > + }
>
> Why don't we just check if f->index is within the [0, ARRAY_SIZE()-1] bounds
> and then just use it to index the array directly?
>
I will refine as :
static int mtk_fd_m2m_enum_fmt_out_mp(struct file *file, void *fh,
struct v4l2_fmtdesc *f)
{
if ((f->index >= 0) && (f->index < NUM_FORMATS)) {
f->pixelformat = in_img_fmts[f->index].pixelformat;
return 0;
}
return -EINVAL;
}
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int mtk_fd_m2m_try_fmt_out_mp(struct file *file,
> > + void *fh,
> > + struct v4l2_format *f)
>
> I think we could just shorten the function prefixes to "mtk_fd_".
>
Do you mean by replace mtk_fd_m2m_* with mtk_fd_ ?
> > +{
> > + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > + const struct v4l2_pix_format_mplane *fmt;
> > +
> > + fmt = mtk_fd_find_fmt(pix_mp->pixelformat);
> > + if (!fmt) {
> > + /* Get default img fmt */
> > + fmt = &in_img_fmts[0];
> > + f->fmt.pix.pixelformat = fmt->pixelformat;
> > + }
> > +
> > + /* Use default */
> > + pix_mp->field = fmt->field;
> > + pix_mp->colorspace = fmt->colorspace;
> > + pix_mp->num_planes = fmt->num_planes;
> > + pix_mp->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > + pix_mp->quantization = V4L2_QUANTIZATION_DEFAULT;
> > + pix_mp->xfer_func =
> > + V4L2_MAP_XFER_FUNC_DEFAULT(pix_mp->colorspace);
> > +
> > + /* Keep user setting as possible */
> > + pix_mp->width = clamp(pix_mp->width,
> > + MTK_FD_OUTPUT_MIN_WIDTH,
> > + MTK_FD_OUTPUT_MAX_WIDTH);
> > + pix_mp->height = clamp(pix_mp->height,
> > + MTK_FD_OUTPUT_MIN_HEIGHT,
> > + MTK_FD_OUTPUT_MAX_HEIGHT);
> > +
> > + pix_mp->plane_fmt[0].bytesperline = pix_mp->width * 2;
>
> Please add a comment explaining why this is always 2.
>
Done.
/* 2 bytes per pixel */
> > + pix_mp->plane_fmt[0].sizeimage =
> > + pix_mp->plane_fmt[0].bytesperline * pix_mp->height;
> > + memset(pix_mp->plane_fmt[0].reserved, 0,
> > + sizeof(pix_mp->plane_fmt[0].reserved));
>
> No need to zero this here. The core will handle it.
>
Ok, I will remove it.
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_m2m_g_fmt_out_mp(struct file *file, void *fh,
> > + struct v4l2_format *f)
> > +{
> > + struct mtk_fd_ctx *ctx = fh_to_ctx(fh);
> > +
> > + f->fmt.pix_mp = ctx->src_fmt;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_m2m_s_fmt_out_mp(struct file *file, void *fh,
> > + struct v4l2_format *f)
> > +{
> > + struct mtk_fd_ctx *ctx = fh_to_ctx(fh);
> > + struct vb2_queue *vq;
> > +
> > + /* Change not allowed if queue is streaming. */
> > + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > + if (vb2_is_streaming(vq) || vb2_is_busy(vq)) {
>
> vb2_is_busy() is a superset of vb2_is_streaming(), so it's enough to just
> check the former.
>
Ok, I will just check the former.
> > + dev_dbg(ctx->dev, "vb2_is_streaming or vb2_is_busy");
> > + return -EBUSY;
> > + }
> > +
> > + mtk_fd_m2m_try_fmt_out_mp(file, fh, f);
> > + ctx->src_fmt = f->fmt.pix_mp;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_m2m_enum_fmt_meta_cap(struct file *file, void *fh,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + if (f->index)
> > + return -EINVAL;
> > +
> > + strscpy(f->description, "Face detection result",
> > + sizeof(f->description));
> > + f->pixelformat = fw_param_fmts[0].dataformat;
> > + f->flags = 0;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_m2m_g_fmt_meta_cap(struct file *file, void *fh,
> > + struct v4l2_format *f)
> > +{
> > + f->fmt.meta.dataformat = fw_param_fmts[0].dataformat;
> > + f->fmt.meta.buffersize = fw_param_fmts[0].buffersize;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct vb2_ops mtk_fd_vb2_ops = {
> > + .queue_setup = mtk_fd_vb2_queue_setup,
> > + .buf_out_validate = mtk_fd_vb2_buf_out_validate,
> > + .buf_prepare = mtk_fd_vb2_buf_prepare,
> > + .buf_queue = mtk_fd_vb2_buf_queue,
> > + .start_streaming = mtk_fd_vb2_start_streaming,
> > + .stop_streaming = mtk_fd_vb2_stop_streaming,
> > + .wait_prepare = vb2_ops_wait_prepare,
> > + .wait_finish = vb2_ops_wait_finish,
> > + .buf_request_complete = mtk_fd_vb2_request_complete,
> > +};
> > +
> > +static const struct v4l2_ioctl_ops mtk_fd_v4l2_video_out_ioctl_ops = {
> > + .vidioc_querycap = mtk_fd_m2m_querycap,
> > + .vidioc_enum_fmt_vid_out_mplane = mtk_fd_m2m_enum_fmt_out_mp,
> > + .vidioc_g_fmt_vid_out_mplane = mtk_fd_m2m_g_fmt_out_mp,
> > + .vidioc_s_fmt_vid_out_mplane = mtk_fd_m2m_s_fmt_out_mp,
> > + .vidioc_try_fmt_vid_out_mplane = mtk_fd_m2m_try_fmt_out_mp,
> > + .vidioc_enum_fmt_meta_cap = mtk_fd_m2m_enum_fmt_meta_cap,
> > + .vidioc_g_fmt_meta_cap = mtk_fd_m2m_g_fmt_meta_cap,
> > + .vidioc_s_fmt_meta_cap = mtk_fd_m2m_g_fmt_meta_cap,
> > + .vidioc_try_fmt_meta_cap = mtk_fd_m2m_g_fmt_meta_cap,
> > + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> > + .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
> > + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
> > + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
> > + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
> > + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
> > + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
> > + .vidioc_streamon = v4l2_m2m_ioctl_streamon,
> > + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
> > + .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> > + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> > +};
> > +
> > +static int
> > +mtk_fd_queue_init(void *priv, struct vb2_queue *src_vq,
> > + struct vb2_queue *dst_vq)
> > +{
> > + struct mtk_fd_ctx *ctx = priv;
> > + int ret;
> > +
> > + src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> > + src_vq->supports_requests = true;
> > + src_vq->drv_priv = ctx;
> > + src_vq->ops = &mtk_fd_vb2_ops;
> > + src_vq->mem_ops = &vb2_dma_contig_memops;
> > + src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> > + src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > + src_vq->lock = &ctx->fd_dev->vfd_lock;
> > + src_vq->dev = ctx->fd_dev->v4l2_dev.dev;
> > +
> > + ret = vb2_queue_init(src_vq);
> > + if (ret)
> > + return ret;
> > +
> > + dst_vq->type = V4L2_BUF_TYPE_META_CAPTURE;
> > + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> > + dst_vq->drv_priv = ctx;
> > + dst_vq->ops = &mtk_fd_vb2_ops;
> > + dst_vq->mem_ops = &vb2_dma_contig_memops;
> > + dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> > + dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > + dst_vq->lock = &ctx->fd_dev->vfd_lock;
> > + dst_vq->dev = ctx->fd_dev->v4l2_dev.dev;
> > +
> > + return vb2_queue_init(dst_vq);
> > +}
> > +
> > +static int mtk_fd_dev_g_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct mtk_fd_ctx *ctx = ctrl->priv;
> > + int i;
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_MTK_FD_SCALE_IMG_WIDTH:
> > + for (i = 0; i < ctrl->elems; i++)
> > + ctrl->p_new.p_u16[i] =
> > + ctx->user_param.scale_img_width[i];
> > + break;
> > + case V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT:
> > + for (i = 0; i < ctrl->elems; i++)
> > + ctrl->p_new.p_u16[i] =
> > + ctx->user_param.scale_img_height[i];
> > + break;
> > + case V4L2_CID_MTK_FD_DETECT_POSE:
> > + ctrl->val = ctx->user_param.fd_pose;
> > + break;
> > + case V4L2_CID_MTK_FD_DETECT_SPEEDUP:
> > + ctrl->val = ctx->user_param.fd_speedup;
> > + break;
> > + case V4L2_CID_MTK_FD_SCALE_IMG_NUM:
> > + ctrl->val = ctx->user_param.scale_img_num;
> > + break;
> > + case V4L2_CID_MTK_FD_EXTRA_MODEL:
> > + ctrl->val = ctx->user_param.fd_extra_model;
> > + break;
> > + default:
> > + dev_dbg(ctx->dev, "%s: unexpected control: 0x%x:%d",
> > + __func__, ctrl->id, ctrl->val);
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_dev_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct mtk_fd_ctx *ctx = ctrl->priv;
> > + int i;
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_MTK_FD_SCALE_IMG_WIDTH:
> > + for (i = 0; i < ctrl->elems; i++)
> > + ctx->user_param.scale_img_width[i] =
> > + ctrl->p_new.p_u16[i];
> > + break;
> > + case V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT:
> > + for (i = 0; i < ctrl->elems; i++)
> > + ctx->user_param.scale_img_height[i] =
> > + ctrl->p_new.p_u16[i];
> > + break;
> > + case V4L2_CID_MTK_FD_DETECT_POSE:
> > + ctx->user_param.fd_pose = ctrl->val;
> > + break;
> > + case V4L2_CID_MTK_FD_DETECT_SPEEDUP:
> > + ctx->user_param.fd_speedup = ctrl->val;
> > + break;
> > + case V4L2_CID_MTK_FD_SCALE_IMG_NUM:
> > + ctx->user_param.scale_img_num = ctrl->val;
> > + break;
> > + case V4L2_CID_MTK_FD_EXTRA_MODEL:
> > + ctx->user_param.fd_extra_model = ctrl->val;
> > + break;
> > + default:
> > + dev_dbg(ctx->dev, "%s: unexpected control: 0x%x:%d",
> > + __func__, ctrl->id, ctrl->val);
> > + return -EINVAL;
>
> No need to handle this, as the framework will only pass the controls we
> registered earlier to this function.
>
Ok, I will remove this .
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mtk_fd_dev_ctrl_ops = {
> > + .g_volatile_ctrl = mtk_fd_dev_g_ctrl,
>
> I don't see any volatile controls below. No need to implement this callback.
>
Ok, I will remove this .
> > + .s_ctrl = mtk_fd_dev_s_ctrl,
>
> I think you might not even need to implement this function (or just provide
> a dummy one that returns 0 if its required) if you just use the controls
> directly when preparing the job for the hardware. Check v4l2_ctrl_find().
>
I will remove the mtk_fd_dev_ctrl_ops, and use the following function to
get control values when preparing the job for HW,
static void mtk_fd_fill_user_param(struct user_param *user_param,
struct v4l2_ctrl_handler *hdl)
{
struct v4l2_ctrl *ctrl;
int i;
ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_SCALE_IMG_WIDTH);
if (ctrl)
for (i = 0; i < ctrl->elems; i++)
user_param->scale_img_width[i] = ctrl->p_new.p_u16[i];
ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT);
if (ctrl)
for (i = 0; i < ctrl->elems; i++)
user_param->scale_img_height[i] = ctrl->p_new.p_u16[i];
ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_DETECT_POSE);
if (ctrl)
user_param->fd_pose = ctrl->val;
pr_info("pose:%d\n", user_param->fd_pose);
ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_DETECT_SPEEDUP);
if (ctrl)
user_param->fd_speedup = ctrl->val;
ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_SCALE_IMG_NUM);
if (ctrl)
user_param->scale_img_num = ctrl->val;
ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_EXTRA_MODEL);
if (ctrl)
user_param->fd_extra_model = ctrl->val;
}
> > +};
> > +
> > +struct v4l2_ctrl_config mtk_fd_controls[] = {
> > + {
> > + .ops = &mtk_fd_dev_ctrl_ops,
> > + .id = V4L2_CID_MTK_FD_SCALE_IMG_WIDTH,
> > + .name = "FD scale image widths",
> > + .type = V4L2_CTRL_TYPE_U16,
> > + .min = MTK_FD_OUTPUT_MIN_WIDTH,
> > + .max = MTK_FD_OUTPUT_MAX_WIDTH,
> > + .step = 1,
> > + .def = MTK_FD_OUTPUT_MAX_WIDTH,
> > + .dims = { FD_SCALE_NUM },
>
> Something wrong with indentation here.
>
Done.
> > + },
> > + {
> > + .ops = &mtk_fd_dev_ctrl_ops,
> > + .id = V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT,
> > + .name = "FD scale image heights",
> > + .type = V4L2_CTRL_TYPE_U16,
> > + .min = MTK_FD_OUTPUT_MIN_HEIGHT,
> > + .max = MTK_FD_OUTPUT_MAX_HEIGHT,
> > + .step = 1,
> > + .def = MTK_FD_OUTPUT_MAX_HEIGHT,
> > + .dims = { FD_SCALE_NUM },
> > + },
> > + {
> > + .ops = &mtk_fd_dev_ctrl_ops,
> > + .id = V4L2_CID_MTK_FD_SCALE_IMG_NUM,
> > + .name = "FD scale size counts",
> > + .type = V4L2_CTRL_TYPE_INTEGER,
> > + .min = 0,
> > + .max = FACE_SIZE_NUM_MAX,
> > + .step = 1,
> > + .def = 0,
> > + },
> > + {
> > + .ops = &mtk_fd_dev_ctrl_ops,
> > + .id = V4L2_CID_MTK_FD_DETECT_POSE,
> > + .name = "FD detect face pose",
> > + .type = V4L2_CTRL_TYPE_INTEGER,
> > + .min = 0,
> > + .max = 3,
> > + .step = 1,
> > + .def = 0,
> > + },
> > + {
> > + .ops = &mtk_fd_dev_ctrl_ops,
> > + .id = V4L2_CID_MTK_FD_DETECT_SPEEDUP,
> > + .name = "FD detection speedup",
> > + .type = V4L2_CTRL_TYPE_INTEGER,
> > + .min = 0,
> > + .max = FD_MAX_SPEEDUP,
> > + .step = 1,
> > + .def = 0,
> > + },
> > + {
> > + .ops = &mtk_fd_dev_ctrl_ops,
> > + .id = V4L2_CID_MTK_FD_EXTRA_MODEL,
> > + .name = "FD use extra model",
> > + .type = V4L2_CTRL_TYPE_BOOLEAN,
> > + .min = 0,
> > + .max = 1,
> > + .step = 1,
> > + .def = 0,
> > + },
> > +};
> > +
> > +static int mtk_fd_ctrls_setup(struct mtk_fd_ctx *ctx)
> > +{
> > + struct v4l2_ctrl_handler *hdl = &ctx->hdl;
> > + struct v4l2_ctrl *ctl;
> > + int i;
> > +
> > + v4l2_ctrl_handler_init(hdl, V4L2_CID_MTK_FD_MAX);
> > + if (hdl->error)
> > + return hdl->error;
> > +
> > + for (i = 0; i < ARRAY_SIZE(mtk_fd_controls); i++) {
> > + ctl = v4l2_ctrl_new_custom(hdl, &mtk_fd_controls[i], ctx);
> > + if (hdl->error) {
> > + v4l2_ctrl_handler_free(hdl);
> > + dev_err(ctx->dev, "Failed to register controls:%d", i);
> > + return hdl->error;
> > + }
> > + }
> > +
> > + ctx->fh.ctrl_handler = &ctx->hdl;
> > + v4l2_ctrl_handler_setup(hdl);
> > +
> > + return 0;
> > +}
> > +
> > +static unsigned int get_fd_img_fmt(unsigned int fourcc)
> > +{
> > + switch (fourcc) {
> > + case V4L2_PIX_FMT_VYUY:
> > + return FMT_VYUY;
> > + case V4L2_PIX_FMT_YUYV:
> > + return FMT_YUYV;
> > + case V4L2_PIX_FMT_YVYU:
> > + return FMT_YVYU;
> > + case V4L2_PIX_FMT_UYVY:
> > + return FMT_UYVY;
> > + default:
> > + return FMT_UNKNOWN;
> > + }
> > +}
> > +
> > +static void init_ctx_fmt(struct mtk_fd_ctx *ctx)
> > +{
> > + const struct v4l2_pix_format_mplane *fmt;
> > +
> > + /* Initialize M2M source fmt */
> > + fmt = &in_img_fmts[0];
> > + mtk_fd_fill_pixfmt_mp(&ctx->src_fmt, fmt);
> > +
> > + /* Initialize M2M destination fmt */
> > + ctx->dst_fmt.buffersize = fw_param_fmts[0].buffersize;
> > + ctx->dst_fmt.dataformat = fw_param_fmts[0].dataformat;
>
> Why not just make dst_fmt a pointer and assign &fw_params_fmt[0] there?
>
fw_params_fmt[] will be removed and assign the const value here.
ctx->dst_fmt.buffersize = sizeof(struct fd_user_output);
ctx->dst_fmt.dataformat = V4L2_META_FMT_MTFD_RESULT;
> > +}
> > +
> > +/*
> > + * V4L2 file operations.
> > + */
> > +static int mtk_vfd_open(struct file *filp)
> > +{
> > + struct mtk_fd_dev *fd_dev = video_drvdata(filp);
> > + struct video_device *vdev = video_devdata(filp);
> > + struct mtk_fd_ctx *ctx;
> > + int ret;
> > +
> > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> > + if (!ctx)
> > + return -ENOMEM;
> > +
> > + ctx->fd_dev = fd_dev;
> > + ctx->dev = &fd_dev->pdev->dev;
> > +
> > + v4l2_fh_init(&ctx->fh, vdev);
> > + filp->private_data = &ctx->fh;
> > +
> > + init_ctx_fmt(ctx);
> > +
> > + ret = mtk_fd_ctrls_setup(ctx);
> > + if (ret) {
> > + dev_err(ctx->dev, "Failed to set up controls:%d\n", ret);
> > + goto err_fh_ctrl;
> > + }
> > +
> > + ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(fd_dev->m2m_dev, ctx,
> > + &mtk_fd_queue_init);
> > + if (IS_ERR(ctx->fh.m2m_ctx)) {
> > + ret = PTR_ERR(ctx->fh.m2m_ctx);
> > + goto err_init_ctx;
> > + }
> > +
> > + v4l2_fh_add(&ctx->fh);
> > +
> > + return 0;
> > +
> > +err_init_ctx:
> > + v4l2_ctrl_handler_free(&ctx->hdl);
> > +err_fh_ctrl:
> > + v4l2_fh_exit(&ctx->fh);
> > + kfree(ctx);
> > +
> > + return ret;
> > +}
> > +
> > +static int mtk_vfd_release(struct file *filp)
> > +{
> > + struct mtk_fd_ctx *ctx = container_of(filp->private_data,
> > + struct mtk_fd_ctx, fh);
> > +
> > + v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
> > +
> > + v4l2_ctrl_handler_free(&ctx->hdl);
> > + v4l2_fh_del(&ctx->fh);
> > + v4l2_fh_exit(&ctx->fh);
> > +
> > + kfree(ctx);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_file_operations fd_video_fops = {
> > + .owner = THIS_MODULE,
> > + .open = mtk_vfd_open,
> > + .release = mtk_vfd_release,
> > + .poll = v4l2_m2m_fop_poll,
> > + .unlocked_ioctl = video_ioctl2,
> > + .mmap = v4l2_m2m_fop_mmap,
>
> We also need the compat_ioctl here for 32-bit userspace on 64-bit kernels.
>
Done.
> > +};
> > +
> > +static void mtk_fd_device_run(void *priv)
> > +{
> > + struct mtk_fd_ctx *ctx = priv;
> > + struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > + struct fd_hw_param fd_param;
> > + void *fd_result_vaddr;
> > +
> > + src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > + dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > +
> > + memset(&fd_param, 0, sizeof(fd_param));
> > +
> > + fd_param.src_img.dma_addr =
> > + vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
> > + fd_param.user_result.dma_addr =
> > + vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
> > + fd_result_vaddr = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
> > +
> > + ctx->user_param.src_img_fmt = get_fd_img_fmt(ctx->src_fmt.pixelformat);
> > + memcpy(&fd_param.user_param, &ctx->user_param, sizeof(ctx->user_param));
> > +
> > + /* Complete request controls if any */
> > + v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, &ctx->hdl);
> > +
> > + mtk_fd_hw_job_exec(&ctx->fd_dev->fd_hw, &fd_param, fd_result_vaddr);
> > +}
> > +
> > +static struct v4l2_m2m_ops fd_m2m_ops = {
> > + .device_run = mtk_fd_device_run,
> > +};
> > +
> > +static int mtk_fd_request_validate(struct media_request *req)
> > +{
> > + unsigned int count;
> > +
> > + count = vb2_request_buffer_cnt(req);
> > + if (!count)
> > + return -ENOENT;
>
> Why -ENOENT?
>
Reference the return code in vb2_request_validate()
I consider refining as following:
static int mtk_fd_request_validate(struct media_request *req)
{
if (vb2_request_buffer_cnt(req) > 1)
return -EINVAL;
return vb2_request_validate(req);
}
or maybe I don't need to worry the request count greater than 1,
just remove this function and set vb2_request_validate as .req_validate
directly?
> > + else if (count > 1)
> > + return -EINVAL;
> > +
> > + return vb2_request_validate(req);
> > +}
> > +
> > +static const struct media_device_ops fd_m2m_media_ops = {
> > + .req_validate = mtk_fd_request_validate,
> > + .req_queue = v4l2_m2m_request_queue,
> > +};
> > +
> > +static int mtk_fd_video_device_register(struct mtk_fd_dev *fd_dev)
> > +{
> > + struct video_device *vfd = &fd_dev->vfd;
> > + struct v4l2_m2m_dev *m2m_dev = fd_dev->m2m_dev;
> > + struct device *dev = &fd_dev->pdev->dev;
> > + int function, ret;
> > +
> > + vfd->fops = &fd_video_fops;
> > + vfd->release = video_device_release;
> > + vfd->lock = &fd_dev->vfd_lock;
> > + vfd->v4l2_dev = &fd_dev->v4l2_dev;
> > + vfd->vfl_dir = VFL_DIR_M2M;
> > + vfd->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_OUTPUT_MPLANE |
> > + V4L2_CAP_META_CAPTURE;
> > + vfd->ioctl_ops = &mtk_fd_v4l2_video_out_ioctl_ops;
> > +
> > + strscpy(vfd->name, "MTK-FD-V4L2", sizeof(vfd->name));
>
> Please use dev_driver_string().
>
Done.
> > + video_set_drvdata(vfd, fd_dev);
> > +
> > + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> > + if (ret) {
> > + dev_err(dev, "Failed to register video device\n");
> > + goto err_free_dev;
> > + }
> > +
> > + function = MEDIA_ENT_F_PROC_VIDEO_DECODER;
>
> MEDIA_ENT_F_PROC_VIDEO_STATISTICS fits here much more.
>
> Also nit: Any reason to have this assigned to this intermediate variable
> rather than just directly passed to the function?
>
Too long for 80 columns, I will remove the intermediate variable and
refine as following:
ret = v4l2_m2m_register_media_controller(m2m_dev, vfd,
MEDIA_ENT_F_PROC_VIDEO_STATISTICS);
> > + ret = v4l2_m2m_register_media_controller(m2m_dev, vfd, function);
> > + if (ret) {
> > + dev_err(dev, "Failed to init mem2mem media controller\n");
> > + goto err_unreg_video;
> > + }
> > + return 0;
> > +
> > +err_unreg_video:
> > + video_unregister_device(vfd);
> > +err_free_dev:
> > + video_device_release(vfd);
> > + return ret;
> > +}
> > +
> > +static int mtk_fd_dev_v4l2_init(struct mtk_fd_dev *fd_dev)
> > +{
> > + struct media_device *mdev = &fd_dev->mdev;
> > + struct device *dev = &fd_dev->pdev->dev;
> > + int ret;
> > +
> > + ret = v4l2_device_register(dev, &fd_dev->v4l2_dev);
> > + if (ret) {
> > + dev_err(dev, "Failed to register v4l2 device\n");
> > + return ret;
> > + }
> > +
> > + fd_dev->m2m_dev = v4l2_m2m_init(&fd_m2m_ops);
> > + if (IS_ERR(fd_dev->m2m_dev)) {
> > + dev_err(dev, "Failed to init mem2mem device\n");
> > + ret = PTR_ERR(fd_dev->m2m_dev);
> > + goto fail_m2m_dev;
>
> Please name the labels after the next clean-up step to be done, not the
> failure.
>
Ok, I will refine as following:
err_unreg_vdev:
v4l2_m2m_unregister_media_controller(fd_dev->m2m_dev);
video_unregister_device(&fd_dev->vfd);
video_device_release(&fd_dev->vfd);
err_cleanup_mdev:
media_device_cleanup(mdev);
v4l2_m2m_release(fd_dev->m2m_dev);
err_unreg_v4l2_dev:
v4l2_device_unregister(&fd_dev->v4l2_dev);
> > + }
> > +
> > + mdev->dev = dev;
> > + strscpy(mdev->model, "MTK-FD-V4L2", sizeof(mdev->model));
>
> Could we just use dev_driver_string() here instead?
>
Done.
> > + snprintf(mdev->bus_info, sizeof(mdev->bus_info),
> > + "platform:%s", dev_name(dev));
> > + media_device_init(mdev);
> > + mdev->ops = &fd_m2m_media_ops;
> > + fd_dev->v4l2_dev.mdev = mdev;
> > +
> > + ret = mtk_fd_video_device_register(fd_dev);
> > + if (ret) {
> > + dev_err(dev, "Failed to register video device\n");
> > + goto err_vdev;
> > + }
> > +
> > + ret = media_device_register(mdev);
> > + if (ret) {
> > + dev_err(dev, "Failed to register mem2mem media device\n");
> > + goto fail_mdev;
> > + }
> > +
> > + return 0;
> > +
> > +fail_mdev:
> > + v4l2_m2m_unregister_media_controller(fd_dev->m2m_dev);
> > + video_unregister_device(&fd_dev->vfd);
> > + video_device_release(&fd_dev->vfd);
> > +err_vdev:
> > + media_device_cleanup(mdev);
> > + v4l2_m2m_release(fd_dev->m2m_dev);
> > +fail_m2m_dev:
> > + v4l2_device_unregister(&fd_dev->v4l2_dev);
> > + return ret;
> > +}
> > +
> > +static void mtk_fd_dev_v4l2_release(struct mtk_fd_dev *fd_dev)
> > +{
> > + v4l2_m2m_unregister_media_controller(fd_dev->m2m_dev);
> > + video_unregister_device(&fd_dev->vfd);
> > + video_device_release(&fd_dev->vfd);
> > + media_device_cleanup(&fd_dev->mdev);
> > + v4l2_m2m_release(fd_dev->m2m_dev);
> > + v4l2_device_unregister(&fd_dev->v4l2_dev);
> > +}
> > +
> > +static irqreturn_t mtk_fd_irq(int irq, void *data)
> > +{
> > + struct mtk_fd_hw *fd_hw = (struct mtk_fd_hw *)data;
> > +
> > + fd_hw->fd_irq_result = readl(fd_hw->fd_base + FD_INT);
>
> We should be able to just handle the job completion directly from here.
>
Ok, I will try to handle the job completion from here.
> > + wake_up_interruptible(&fd_hw->wq);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int mtk_fd_hw_get_scp_mem(struct mtk_fd_hw *fd_hw,
> > + struct fd_buffer *scp_mem)
> > +{
> > + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> > + struct device *dev = &fd_dev->pdev->dev;
> > + dma_addr_t addr;
> > + u32 size;
> > +
> > + scp_mem->scp_addr = scp_get_reserve_mem_phys(SCP_FD_MEM_ID);
> > + size = scp_get_reserve_mem_size(SCP_FD_MEM_ID);
> > + if (!scp_mem->scp_addr || !size)
> > + return -EPROBE_DEFER;
>
> Why -EPROBE_DEFER? Should we have some way to distinguish between the SCP
> driver not being initialized yet and some other error?
>
Ok, I will fix it to -ENOMEM.
Not being initialized or some other error can be found during device
probe.
(will also load scp stuff there)
> > +
> > + /* get dma addr address */
> > + addr = dma_map_page_attrs(dev, phys_to_page(scp_mem->scp_addr), 0,
> > + size, DMA_BIDIRECTIONAL,
>
> Should this be really bidirectional?
>
I will fix it to DMA_TO_DEVICE
> > + DMA_ATTR_SKIP_CPU_SYNC);
> > + if (dma_mapping_error(dev, addr)) {
> > + scp_mem->scp_addr = 0;
> > + dev_err(dev, "Failed to map scp addr\n");
> > + return -ENOMEM;
> > + }
> > + scp_mem->dma_addr = addr;
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_probe(struct platform_device *pdev)
> > +{
> > + struct mtk_fd_dev *fd_dev;
>
> nit: Perhaps just fd, to have shorter code?
>
Ok, I will refine as
struct mtk_fd_dev *fd;
> > + struct device *dev = &pdev->dev;
> > + struct mtk_fd_hw *fd_hw;
> > + struct resource *res;
> > + int irq;
> > + int ret;
> > +
> > + fd_dev = devm_kzalloc(&pdev->dev, sizeof(*fd_dev), GFP_KERNEL);
> > + if (!fd_dev)
> > + return -ENOMEM;
> > +
> > + dev_set_drvdata(dev, fd_dev);
> > + fd_hw = &fd_dev->fd_hw;
> > + fd_dev->pdev = pdev;
>
> Is pdev useful for anything? Perhaps you want to store &pdev->dev instead?
>
Yes, I will store that instead.
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + dev_err(dev, "no IRQ:%d resource info\n", irq);
> > + return irq;
> > + }
> > + ret = devm_request_irq(dev, irq, mtk_fd_irq, IRQF_SHARED,
> > + dev_driver_string(dev),
> > + fd_hw);
>
> Why not just fd_dev?
>
Ok, I will use fd_dev here.
> > + if (ret) {
> > + dev_err(dev, "req_irq fail:%d\n", irq);
> > + return ret;
> > + }
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + fd_hw->fd_base = devm_ioremap_resource(dev, res);
> > + if (!fd_hw->fd_base) {
> > + dev_err(dev, "unable to get fd reg base\n");
> > + return PTR_ERR(fd_hw->fd_base);
> > + }
> > +
> > + fd_hw->fd_clk = devm_clk_get(dev, "fd");
> > + if (IS_ERR(fd_hw->fd_clk)) {
> > + dev_err(dev, "cannot get fd_clk_img_fd clock\n");
> > + return PTR_ERR(fd_hw->fd_clk);
> > + }
> > +
> > + ret = mtk_fd_hw_get_scp_mem(fd_hw, &fd_hw->scp_mem);
> > + if (ret) {
> > + dev_err(dev, "scp memory init failed: %d\n", ret);
>
> nit: Could we make the error messages a bit more consistent? For example
> "Failed to get IRQ resource (%d)", "Failed to request IRQ (%d)", "Failed to
> ... (%d)", etc.
>
Ok, I will fix it.
> > + return ret;
> > + }
> > +
> > + atomic_set(&fd_hw->fd_user_cnt, 0);
> > + init_waitqueue_head(&fd_hw->wq);
> > + mutex_init(&fd_dev->vfd_lock);
> > + mutex_init(&fd_hw->fd_hw_lock);
> > + pm_runtime_enable(dev);
> > +
> > + ret = mtk_fd_dev_v4l2_init(fd_dev);
> > + if (ret) {
> > + mutex_destroy(&fd_dev->fd_hw.fd_hw_lock);
> > + mutex_destroy(&fd_dev->vfd_lock);
> > + pm_runtime_disable(&pdev->dev);
>
> Please move the clean-up to an error path on the bottom of the function.
>
Done.
> > + dev_err(dev, "v4l2 init failed: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_remove(struct platform_device *pdev)
> > +{
> > + struct mtk_fd_dev *fd_dev = dev_get_drvdata(&pdev->dev);
> > +
> > + mtk_fd_dev_v4l2_release(fd_dev);
> > + mutex_destroy(&fd_dev->fd_hw.fd_hw_lock);
> > + mutex_destroy(&fd_dev->vfd_lock);
> > + pm_runtime_disable(&pdev->dev);
>
> The order of calls in remove should normally be the opposite to probe.
>
Ok, I will refine as following
mtk_fd_dev_v4l2_release(fd_dev);
pm_runtime_disable(&pdev->dev);
mutex_destroy(&fd_dev->fd_hw.fd_hw_lock);
mutex_destroy(&fd_dev->vfd_lock);
rproc_put(fd_dev->fd_hw.rproc_handle);
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_suspend(struct device *dev)
> > +{
> > + struct mtk_fd_dev *fd_dev = dev_get_drvdata(dev);
> > +
> > + if (pm_runtime_suspended(dev))
> > + return 0;
> > +
> > + /* suspend FD HW */
> > + writel(0x0, fd_dev->fd_hw.fd_base + FD_HW_ENABLE);
> > + writel(0x0, fd_dev->fd_hw.fd_base + FD_INT_EN);
> > + clk_disable_unprepare(fd_dev->fd_hw.fd_clk);
> > + return 0;
> > +}
> > +
> > +static int mtk_fd_resume(struct device *dev)
> > +{
> > + struct mtk_fd_dev *fd_dev = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + if (pm_runtime_suspended(dev))
> > + return 0;
> > +
> > + ret = clk_prepare_enable(fd_dev->fd_hw.fd_clk);
> > + if (ret < 0) {
> > + dev_dbg(dev, "open fd clk failed\n");
> > + clk_disable_unprepare(fd_dev->fd_hw.fd_clk);
> > + }
> > +
> > + /* resume FD HW */
> > + writel(ENABLE_FD, fd_dev->fd_hw.fd_base + FD_HW_ENABLE);
> > + if (fd_dev->fd_hw.state == FD_ENQ)
> > + writel(0x1, fd_dev->fd_hw.fd_base + FD_INT_EN);
> > + return 0;
> > +}
> > +
> > +static const struct dev_pm_ops mtk_fd_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(mtk_fd_suspend, mtk_fd_resume)
> > + SET_RUNTIME_PM_OPS(mtk_fd_suspend, mtk_fd_resume, NULL)
>
> We need separate runtime and system PM ops. Their behavior is expected to be
> different.
>
> For runtime PM ops, you the functions should just unconditionally power on
> or power off the device.
>
> For system PM ops, suspend needs to make sure that no further job is queued
> to the hardware and that any job being already processed by the hardware is
> completed. Resume needs to resume the processing if there are any further
> jobs to be queued to the hardware.
>
Ok, for runtime suspend/resume I will add the following functions:
static int mtk_fd_runtime_suspend(struct device *dev)
{
struct mtk_fd_dev *fd_dev = dev_get_drvdata(dev);
dev_dbg(dev, "%s:disable clock\n", __func__);
clk_disable_unprepare(fd_dev->fd_hw.fd_clk);
return 0;
}
static int mtk_fd_runtime_resume(struct device *dev)
{
struct mtk_fd_dev *fd_dev = dev_get_drvdata(dev);
int ret;
dev_dbg(dev, "%s:enable clock\n", __func__);
ret = clk_prepare_enable(fd_dev->fd_hw.fd_clk);
if (ret < 0) {
dev_err(dev, "Failed to open fd clk:%d\n", ret);
return ret;
}
return 0;
}
Best regards,
Jerry
> > +};
> > +
> > +static const struct of_device_id mtk_fd_of_ids[] = {
> > + { .compatible = "mediatek,mt8183-fd", },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_fd_of_ids);
> > +
> > +static struct platform_driver mtk_fd_driver = {
> > + .probe = mtk_fd_probe,
> > + .remove = mtk_fd_remove,
> > + .driver = {
> > + .name = "mtk-fd-4.0",
> > + .of_match_table = of_match_ptr(mtk_fd_of_ids),
> > + .pm = &mtk_fd_pm_ops,
> > + }
> > +};
> > +module_platform_driver(mtk_fd_driver);
> > +
> > +MODULE_DESCRIPTION("Mediatek FD driver");
> > +MODULE_LICENSE("GPL");
>
> GPL v2
>
> 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] ARM: dts: vf610-zii-scu4-aib: Use generic names for DT nodes
From: Shawn Guo @ 2019-08-25 7:18 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Cory Tusar, Fabio Estevam, Chris Healy, linux-arm-kernel,
linux-kernel
In-Reply-To: <20190824002747.14610-1-andrew.smirnov@gmail.com>
On Fri, Aug 23, 2019 at 05:27:47PM -0700, Andrey Smirnov wrote:
> The devicetree specification recommends using generic node names.
>
> Some ZII dts files already follow such recommendation, but some don't,
> so use generic node names for consistency among the ZII dts files.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Cory Tusar <cory.tusar@zii.aero>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
It doesn't apply to my branch.
Shawn
> ---
> arch/arm/boot/dts/vf610-zii-scu4-aib.dts | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
> index 45a978defbdc..6edd682dbd29 100644
> --- a/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
> +++ b/arch/arm/boot/dts/vf610-zii-scu4-aib.dts
> @@ -417,7 +417,7 @@
> pinctrl-0 = <&pinctrl_dspi1>;
> status = "okay";
>
> - spi-flash@0 {
> + flash@0 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "jedec,spi-nor";
> @@ -430,7 +430,7 @@
> };
> };
>
> - spi-flash@1 {
> + flash@1 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "jedec,spi-nor";
> @@ -519,7 +519,7 @@
> #gpio-cells = <2>;
> };
>
> - lm75@48 {
> + temp-sensor@48 {
> compatible = "national,lm75";
> reg = <0x48>;
> };
> @@ -534,7 +534,7 @@
> reg = <0x52>;
> };
>
> - ds1682@6b {
> + elapsed-time-recorder@6b {
> compatible = "dallas,ds1682";
> reg = <0x6b>;
> };
> @@ -551,7 +551,7 @@
> reg = <0x38>;
> };
>
> - adt7411@4a {
> + adc@4a {
> compatible = "adi,adt7411";
> reg = <0x4a>;
> };
> @@ -563,7 +563,7 @@
> pinctrl-0 = <&pinctrl_i2c2>;
> status = "okay";
>
> - gpio9: sx1503q@20 {
> + gpio9: io-expander@20 {
> compatible = "semtech,sx1503q";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_sx1503_20>;
> @@ -574,12 +574,12 @@
> interrupts = <31 IRQ_TYPE_EDGE_FALLING>;
> };
>
> - lm75@4e {
> + temp-sensor@4e {
> compatible = "national,lm75";
> reg = <0x4e>;
> };
>
> - lm75@4f {
> + temp-sensor@4f {
> compatible = "national,lm75";
> reg = <0x4f>;
> };
> @@ -591,17 +591,17 @@
> reg = <0x23>;
> };
>
> - adt7411@4a {
> + adc@4a {
> compatible = "adi,adt7411";
> reg = <0x4a>;
> };
>
> - at24c08@54 {
> + eeprom@54 {
> compatible = "atmel,24c08";
> reg = <0x54>;
> };
>
> - tca9548@70 {
> + i2c-mux@70 {
> compatible = "nxp,pca9548";
> pinctrl-names = "default";
> #address-cells = <1>;
> @@ -639,7 +639,7 @@
> };
> };
>
> - tca9548@71 {
> + i2c-mux@71 {
> compatible = "nxp,pca9548";
> pinctrl-names = "default";
> reg = <0x71>;
> --
> 2.21.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: [PATCH] ARM: dts: vf610-zii-scu4-aib: Configure IRQ line for GPIO expander
From: Shawn Guo @ 2019-08-25 7:15 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Cory Tusar, Fabio Estevam, Chris Healy, linux-arm-kernel,
linux-kernel
In-Reply-To: <20190824002703.13902-1-andrew.smirnov@gmail.com>
On Fri, Aug 23, 2019 at 05:27:03PM -0700, Andrey Smirnov wrote:
> Configure IRQ line for SX1503 GPIO expander. We already have
> appropriate pinmux entry and all that is missing is "interrupt-parent"
> and "interrupts" properties. Add them.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Cory Tusar <cory.tusar@zii.aero>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
Applied, 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] input: keyboard: snvs_pwrkey: Send press and release event for i.MX6 S,DL and Q
From: Shawn Guo @ 2019-08-25 7:10 UTC (permalink / raw)
To: Robin van der Gracht
Cc: Mark Rutland, devicetree, Adam Ford, Sascha Hauer,
Dmitry Torokhov, linux-kernel, Rob Herring, NXP Linux Team,
Pengutronix Kernel Team, linux-input, Robin Gong, Fabio Estevam,
linux-arm-kernel
In-Reply-To: <20190823123002.10448-1-robin@protonic.nl>
On Fri, Aug 23, 2019 at 02:30:02PM +0200, Robin van der Gracht wrote:
> The older generation i.MX6 processors send a powerdown request interrupt if
> the powerkey is released before a hard shutdown (5 second press). This should
> allow software to bring down the SoC safely.
>
> For this driver to work as a regular powerkey with the older SoCs, we need to
> send a keypress AND release when we get the powerdown request interrupt.
>
> Signed-off-by: Robin van der Gracht <robin@protonic.nl>
> ---
> arch/arm/boot/dts/imx6qdl.dtsi | 2 +-
> arch/arm/boot/dts/imx6sll.dtsi | 2 +-
> arch/arm/boot/dts/imx6sx.dtsi | 2 +-
> arch/arm/boot/dts/imx6ul.dtsi | 2 +-
> arch/arm/boot/dts/imx7s.dtsi | 2 +-
Please have dts changes in a separate patch.
> drivers/input/keyboard/Kconfig | 2 +-
> drivers/input/keyboard/snvs_pwrkey.c | 59 +++++++++++++++++++++++-----
> 7 files changed, 56 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index b3a77bcf00d51..c10d12658743c 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -836,7 +836,7 @@
> };
>
> snvs_pwrkey: snvs-powerkey {
> - compatible = "fsl,sec-v4.0-pwrkey";
> + compatible = "fsl,imx6qdl-sec-v4.0-pwrkey";
> regmap = <&snvs>;
> interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> linux,keycode = <KEY_POWER>;
> diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
> index 1b4899f0fcded..91c7d5bdcc359 100644
> --- a/arch/arm/boot/dts/imx6sll.dtsi
> +++ b/arch/arm/boot/dts/imx6sll.dtsi
> @@ -571,7 +571,7 @@
> };
>
> snvs_pwrkey: snvs-powerkey {
> - compatible = "fsl,sec-v4.0-pwrkey";
> + compatible = "fsl,imx6sx-sec-v4.0-pwrkey";
> regmap = <&snvs>;
> interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> linux,keycode = <KEY_POWER>;
> diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
> index b16a123990a26..b6736db65350f 100644
> --- a/arch/arm/boot/dts/imx6sx.dtsi
> +++ b/arch/arm/boot/dts/imx6sx.dtsi
> @@ -733,7 +733,7 @@
> };
>
> snvs_pwrkey: snvs-powerkey {
> - compatible = "fsl,sec-v4.0-pwrkey";
> + compatible = "fsl,imx6sx-sec-v4.0-pwrkey";
> regmap = <&snvs>;
> interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> linux,keycode = <KEY_POWER>;
> diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
> index a7f6d1d58e20d..d4678c52b55db 100644
> --- a/arch/arm/boot/dts/imx6ul.dtsi
> +++ b/arch/arm/boot/dts/imx6ul.dtsi
> @@ -644,7 +644,7 @@
> };
>
> snvs_pwrkey: snvs-powerkey {
> - compatible = "fsl,sec-v4.0-pwrkey";
> + compatible = "fsl,imx6sx-sec-v4.0-pwrkey";
> regmap = <&snvs>;
> interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> linux,keycode = <KEY_POWER>;
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 106711d2c01b0..bb68c23beb199 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -604,7 +604,7 @@
> };
>
> snvs_pwrkey: snvs-powerkey {
> - compatible = "fsl,sec-v4.0-pwrkey";
> + compatible = "fsl,imx6sx-sec-v4.0-pwrkey";
> regmap = <&snvs>;
> interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> linux,keycode = <KEY_POWER>;
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 7c4f19dab34fd..937e58da5ce17 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -436,7 +436,7 @@ config KEYBOARD_SNVS_PWRKEY
> depends on OF
> help
> This is the snvs powerkey driver for the Freescale i.MX application
> - processors that are newer than i.MX6 SX.
> + processors.
>
> To compile this driver as a module, choose M here; the
> module will be called snvs_pwrkey.
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index 5342d8d45f811..c321e5f2d1087 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -29,6 +29,11 @@
> #define DEBOUNCE_TIME 30
> #define REPEAT_INTERVAL 60
>
> +enum imx_snvs_hwtype {
> + IMX6SX_SNVS, /* i.MX6 SoloX and newer */
> + IMX6QDL_SNVS, /* i.MX6 Solo, DualLite adn Quad */
> +};
> +
> struct pwrkey_drv_data {
> struct regmap *snvs;
> int irq;
> @@ -37,6 +42,19 @@ struct pwrkey_drv_data {
> int wakeup;
> struct timer_list check_timer;
> struct input_dev *input;
> + enum imx_snvs_hwtype hwtype;
> +};
> +
> +static const struct platform_device_id imx_snvs_devtype[] = {
> + {
> + .name = "imx6sx-snvs-pwrkey",
> + .driver_data = IMX6SX_SNVS,
> + }, {
> + .name = "imx6qdl-snvs-pwrkey",
> + .driver_data = IMX6QDL_SNVS,
> + }, {
> + /* sentinel */
> + }
> };
>
> static void imx_imx_snvs_check_for_events(struct timer_list *t)
> @@ -67,13 +85,23 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
> {
> struct platform_device *pdev = dev_id;
> struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
> + struct input_dev *input = pdata->input;
> u32 lp_status;
>
> - pm_wakeup_event(pdata->input->dev.parent, 0);
> + pm_wakeup_event(input->dev.parent, 0);
>
> regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
> - if (lp_status & SNVS_LPSR_SPO)
> - mod_timer(&pdata->check_timer, jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
> + if (lp_status & SNVS_LPSR_SPO) {
> + if (pdata->hwtype == IMX6QDL_SNVS) {
> + input_report_key(input, pdata->keycode, 1);
> + input_report_key(input, pdata->keycode, 0);
> + input_sync(input);
> + pm_relax(input->dev.parent);
> + } else {
> + mod_timer(&pdata->check_timer,
> + jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
> + }
> + }
>
> /* clear SPO status */
> regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO);
> @@ -88,11 +116,24 @@ static void imx_snvs_pwrkey_act(void *pdata)
> del_timer_sync(&pd->check_timer);
> }
>
> +static const struct of_device_id imx_snvs_pwrkey_ids[] = {
> + {
> + .compatible = "fsl,imx6sx-sec-v4.0-pwrkey",
The new compatible should be documented.
> + .data = &imx_snvs_devtype[IMX6SX_SNVS],
> + }, {
> + .compatible = "fsl,imx6qdl-sec-v4.0-pwrkey",
> + .data = &imx_snvs_devtype[IMX6QDL_SNVS],
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
> +
> static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> {
> struct pwrkey_drv_data *pdata = NULL;
> struct input_dev *input = NULL;
> struct device_node *np;
> + const struct of_device_id *of_id;
> int error;
>
> /* Get SNVS register Page */
> @@ -100,6 +141,11 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> if (!np)
> return -ENODEV;
>
> + of_id = of_match_node(imx_snvs_pwrkey_ids, np);
> + if (!of_id)
> + return -ENODEV;
> + pdev->id_entry = of_id->data;
> +
> pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> if (!pdata)
> return -ENOMEM;
> @@ -116,6 +162,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> }
>
> pdata->wakeup = of_property_read_bool(np, "wakeup-source");
> + pdata->hwtype = pdev->id_entry->driver_data;
>
> pdata->irq = platform_get_irq(pdev, 0);
> if (pdata->irq < 0) {
> @@ -175,12 +222,6 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id imx_snvs_pwrkey_ids[] = {
> - { .compatible = "fsl,sec-v4.0-pwrkey" },
The compatible should be kept for not breaking existing DTBs.
Shawn
> - { /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
> -
> static struct platform_driver imx_snvs_pwrkey_driver = {
> .driver = {
> .name = "snvs_pwrkey",
> --
> 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
* [PATCH 7/7] arm64: dts: meson-gxm-khadas-vim2: use rc-khadas keymap
From: Christian Hewitt @ 2019-08-25 4:01 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Kevin Hilman, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Chrisitian Hewitt
In-Reply-To: <1566705688-18442-1-git-send-email-christianshewitt@gmail.com>
Swap to the rc-khadas keymap that maps the mouse button to KEY_MUTE.
Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
index 989d33a..f25ddd1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
@@ -299,7 +299,7 @@
status = "okay";
pinctrl-0 = <&remote_input_ao_pins>;
pinctrl-names = "default";
- linux,rc-map-name = "rc-geekbox";
+ linux,rc-map-name = "rc-khadas";
};
&pwm_AO_ab {
--
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 6/7] arm64: dts: meson-gxl-s905w-tx3-mini: add rc-tx3mini keymap
From: Christian Hewitt @ 2019-08-25 4:01 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Kevin Hilman, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Chrisitian Hewitt
In-Reply-To: <1566705688-18442-1-git-send-email-christianshewitt@gmail.com>
add the rc-tx3mini keymap to the ir node
Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
index 789c819..dd729ac 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts
@@ -20,3 +20,7 @@
reg = <0x0 0x0 0x0 0x40000000>; /* 1 GiB or 2 GiB */
};
};
+
+&ir {
+ linux,rc-map-name = "rc-tanix-tx3mini";
+};
--
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 5/7] arm64: dts: meson-gxl-s905x-khadas-vim: use rc-khadas keymap
From: Christian Hewitt @ 2019-08-25 4:01 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Kevin Hilman, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Chrisitian Hewitt
In-Reply-To: <1566705688-18442-1-git-send-email-christianshewitt@gmail.com>
Swap to the rc-khadas keymap that maps the mouse button to KEY_MUTE.
Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
index 5499e8d..2a5cd30 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
@@ -110,7 +110,7 @@
};
&ir {
- linux,rc-map-name = "rc-geekbox";
+ linux,rc-map-name = "rc-khadas";
};
&gpio_ao {
--
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 4/7] arm64: dts: meson-gxbb-wetek-play2: add rc-wetek-play2 keymap
From: Christian Hewitt @ 2019-08-25 4:01 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Kevin Hilman, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Chrisitian Hewitt
In-Reply-To: <1566705688-18442-1-git-send-email-christianshewitt@gmail.com>
add the rc-wetek-play2 keymap to the ir node
Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts
index 0038522..1d32d1f 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts
@@ -54,3 +54,7 @@
&usb1 {
status = "okay";
};
+
+&ir {
+ linux,rc-map-name = "rc-wetek-play2";
+};
--
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 3/7] arm64: dts: meson-gxbb-wetek-hub: add rc-wetek-hub keymap
From: Christian Hewitt @ 2019-08-25 4:01 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Kevin Hilman, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Chrisitian Hewitt
In-Reply-To: <1566705688-18442-1-git-send-email-christianshewitt@gmail.com>
add the rc-wetek-hub keymap to the ir node
Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts
index 2bfe699..83b985b 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts
@@ -12,3 +12,7 @@
compatible = "wetek,hub", "amlogic,meson-gxbb";
model = "WeTek Hub";
};
+
+&ir {
+ linux,rc-map-name = "rc-wetek-hub";
+};
--
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 2/7] arm64: dts: meson-g12a-x96-max: add rc-x96max keymap
From: Christian Hewitt @ 2019-08-25 4:01 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Kevin Hilman, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Chrisitian Hewitt
In-Reply-To: <1566705688-18442-1-git-send-email-christianshewitt@gmail.com>
add the rc-x96max keymap to the ir node
Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
index fe4013c..357d7dc 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
@@ -277,6 +277,7 @@
status = "okay";
pinctrl-0 = <&remote_input_ao_pins>;
pinctrl-names = "default";
+ linux,rc-map-name = "rc-x96max";
};
&ext_mdio {
--
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 1/7] arm64: dts: meson-g12b-odroid-n2: add rc-odroid keymap
From: Christian Hewitt @ 2019-08-25 4:01 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Kevin Hilman, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Chrisitian Hewitt
In-Reply-To: <1566705688-18442-1-git-send-email-christianshewitt@gmail.com>
add the rc-odroid keymap to the ir node
Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
index 81780ff..35cef76 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
@@ -314,6 +314,7 @@
status = "okay";
pinctrl-0 = <&remote_input_ao_pins>;
pinctrl-names = "default";
+ linux,rc-map-name = "rc-odroid";
};
/* SD card */
--
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 0/7] arm64: dts: meson: ir keymap updates
From: Christian Hewitt @ 2019-08-25 4:01 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Kevin Hilman, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Chrisitian Hewitt
This series adds keymaps for several box/board vendor IR remote devices
to respective device-tree files. The keymaps were submitted in [0] and
have been queued for inclusion in Linux 5.4.
The Khadas remote change swaps the rc-geekbox keymap for rc-khadas. The
Geekbox branded remote was only sold for a brief period when VIM(1) was
a new device. The Khadas branded remote that replaced it exchanged the
Geekbox full-screen key for an Android mouse button using a different IR
keycode. The rc-khadas keymap supports the mouse button keycode and maps
it to KEY_MUTE.
[0] https://patchwork.kernel.org/project/linux-media/list/?series=160309
Christian Hewitt (7):
arm64: dts: meson-g12b-odroid-n2: add rc-odroid keymap
arm64: dts: meson-g12a-x96-max: add rc-x96max keymap
arm64: dts: meson-gxbb-wetek-hub: add rc-wetek-hub keymap
arm64: dts: meson-gxbb-wetek-play2: add rc-wetek-play2 keymap
arm64: dts: meson-gxl-s905x-khadas-vim: use rc-khadas keymap
arm64: dts: meson-gxl-s905w-tx3-mini: add rc-tx3mini keymap
arm64: dts: meson-gxm-khadas-vim2: use rc-khadas keymap
arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts | 1 +
arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts | 1 +
arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-hub.dts | 4 ++++
arch/arm64/boot/dts/amlogic/meson-gxbb-wetek-play2.dts | 4 ++++
arch/arm64/boot/dts/amlogic/meson-gxl-s905w-tx3-mini.dts | 4 ++++
arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts | 2 +-
7 files changed, 16 insertions(+), 2 deletions(-)
--
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
* RE: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a
From: Xiaowei Bao @ 2019-08-25 3:07 UTC (permalink / raw)
To: christophe leroy, Andrew Murray
Cc: mark.rutland@arm.com, Roy Zang, bhelgaas@google.com,
lorenzo.pieralisi@arm.co, arnd@arndb.de,
devicetree@vger.kernel.org, gregkh@linuxfoundation.org, Leo Li,
linux-kernel@vger.kernel.org, kishon@ti.com, M.h. Lian,
robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org,
linux-pci@vger.kernel.org, jingoohan1@gmail.com,
shawnguo@kernel.org, gustavo.pimentel@synopsys.com,
linuxppc-dev@lists.ozlabs.org, Mingkai Hu
In-Reply-To: <89c90732-5e42-f87e-73b1-8d615355afc4@c-s.fr>
> -----Original Message-----
> From: christophe leroy <christophe.leroy@c-s.fr>
> Sent: 2019年8月24日 14:45
> To: Xiaowei Bao <xiaowei.bao@nxp.com>; Andrew Murray
> <andrew.murray@arm.com>
> Cc: mark.rutland@arm.com; Roy Zang <roy.zang@nxp.com>;
> lorenzo.pieralisi@arm.co; arnd@arndb.de; devicetree@vger.kernel.org;
> gregkh@linuxfoundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; kishon@ti.com; M.h.
> Lian <minghuan.lian@nxp.com>; robh+dt@kernel.org;
> gustavo.pimentel@synopsys.com; jingoohan1@gmail.com;
> bhelgaas@google.com; Leo Li <leoyang.li@nxp.com>; shawnguo@kernel.org;
> Mingkai Hu <mingkai.hu@nxp.com>; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
>
>
>
> Le 24/08/2019 à 02:18, Xiaowei Bao a écrit :
> >
> >
> >> -----Original Message-----
> >> From: Andrew Murray <andrew.murray@arm.com>
> >> Sent: 2019年8月23日 22:28
> >> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> >> Cc: bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> >> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> >> lorenzo.pieralisi@arm.co; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h.
> >> Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy
> >> Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
> >> gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> >> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> >> Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support
> >> for ls1088a and ls2088a
> >>
> >> On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> >>> Add PCIe EP mode support for ls1088a and ls2088a, there are some
> >>> difference between LS1 and LS2 platform, so refactor the code of the
> >>> EP driver.
> >>>
> >>> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> >>> ---
> >>> v2:
> >>> - New mechanism for layerscape EP driver.
> >>
> >> Was there a v1 of this patch?
> >
> > Yes, but I don't know how to comments, ^_^
>
> As far as I can see, in the previous version of the series
> (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.ozlabs.org%2Fproject%2Flinuxppc-dev%2Flist%2F%3Fseries%3D125315
> %26state%3D*&data=02%7C01%7Cxiaowei.bao%40nxp.com%7C1befe9
> a67c8046f9535e08d7285eaab6%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C637022259387139020&sdata=p4wbycd04Z7qRUfAoZtwc
> UP7pR%2FuA3%2FjVcWMz6YyQVQ%3D&reserved=0),
> the 8/10 was something completely different, and I can't find any other patch
> in the series that could have been the v1 of this patch.
Thanks, I will correct it to v1 in next version patch.
>
> Christophe
>
> >
> >>
> >>>
> >>> drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> >>> ++++++++++++++++++++------
> >>> 1 file changed, 58 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> index 7ca5fe8..2a66f07 100644
> >>> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> @@ -20,27 +20,29 @@
> >>>
> >>> #define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
> >>>
> >>> -struct ls_pcie_ep {
> >>> - struct dw_pcie *pci;
> >>> - struct pci_epc_features *ls_epc;
> >>> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> >>> +
> >>> +struct ls_pcie_ep_drvdata {
> >>> + u32 func_offset;
> >>> + const struct dw_pcie_ep_ops *ops;
> >>> + const struct dw_pcie_ops *dw_pcie_ops;
> >>> };
> >>>
> >>> -#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> >>> +struct ls_pcie_ep {
> >>> + struct dw_pcie *pci;
> >>> + struct pci_epc_features *ls_epc;
> >>> + const struct ls_pcie_ep_drvdata *drvdata; };
> >>>
> >>> static int ls_pcie_establish_link(struct dw_pcie *pci) {
> >>> return 0;
> >>> }
> >>>
> >>> -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> >>> +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> >>> .start_link = ls_pcie_establish_link, };
> >>>
> >>> -static const struct of_device_id ls_pcie_ep_of_match[] = {
> >>> - { .compatible = "fsl,ls-pcie-ep",},
> >>> - { },
> >>> -};
> >>> -
> >>> static const struct pci_epc_features*
> >>> ls_pcie_ep_get_features(struct dw_pcie_ep *ep) { @@ -82,10 +84,44
> >>> @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> >>> }
> >>> }
> >>>
> >>> -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> >>> +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> >>> + u8 func_no)
> >>> +{
> >>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >>> + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> >>> + u8 header_type;
> >>> +
> >>> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> >>> +
> >>> + if (header_type & (1 << 7))
> >>> + return pcie->drvdata->func_offset * func_no;
> >>> + else
> >>> + return 0;
> >>
> >> It looks like there isn't a PCI define for multi function, the
> >> nearest I could find was PCI_HEADER_TYPE_MULTIDEVICE in
> >> hotplug/ibmphp.h. A comment above the test might be helpful to explain
> the test.
> >
> > Yes, I have not find the PCI_HEADER_TYPE_MULTIDEVICE define. OK, I
> > will add The comments in next version patch.
> >
> >>
> >> As the ls_pcie_ep_drvdata structures are static, the unset
> >> .func_offset will be initialised to 0, so you could just drop the test above.
> >
> > OK, thanks
> >
> >>
> >> However something to the effect of the following may help spot
> >> misconfiguration:
> >>
> >> WARN_ON(func_no && !pcie->drvdata->func_offset); return
> >> pcie->drvdata->func_offset * func_no;
> >
> > Thanks a lot, this looks better.
> >
> >>
> >> The WARN is probably quite useful as if you are attempting to use
> >> non-zero functions and func_offset isn't set - then things may appear
> >> to work normally but actually will break horribly.
> >
> > got it, thanks.
> >
> >>
> >> Thanks,
> >>
> >> Andrew Murray
> >>
> >>> +}
> >>> +
> >>> +static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
> >>> .ep_init = ls_pcie_ep_init,
> >>> .raise_irq = ls_pcie_ep_raise_irq,
> >>> .get_features = ls_pcie_ep_get_features,
> >>> + .func_conf_select = ls_pcie_ep_func_conf_select, };
> >>> +
> >>> +static const struct ls_pcie_ep_drvdata ls1_ep_drvdata = {
> >>> + .ops = &ls_pcie_ep_ops,
> >>> + .dw_pcie_ops = &dw_ls_pcie_ep_ops, };
> >>> +
> >>> +static const struct ls_pcie_ep_drvdata ls2_ep_drvdata = {
> >>> + .func_offset = 0x20000,
> >>> + .ops = &ls_pcie_ep_ops,
> >>> + .dw_pcie_ops = &dw_ls_pcie_ep_ops, };
> >>> +
> >>> +static const struct of_device_id ls_pcie_ep_of_match[] = {
> >>> + { .compatible = "fsl,ls1046a-pcie-ep", .data = &ls1_ep_drvdata },
> >>> + { .compatible = "fsl,ls1088a-pcie-ep", .data = &ls2_ep_drvdata },
> >>> + { .compatible = "fsl,ls2088a-pcie-ep", .data = &ls2_ep_drvdata },
> >>> + { },
> >>> };
> >>>
> >>> static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie, @@ -98,7
> >>> +134,7 @@ static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> >>> int ret;
> >>>
> >>> ep = &pci->ep;
> >>> - ep->ops = &pcie_ep_ops;
> >>> + ep->ops = pcie->drvdata->ops;
> >>>
> >>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> >> "addr_space");
> >>> if (!res)
> >>> @@ -137,14 +173,11 @@ static int __init ls_pcie_ep_probe(struct
> >> platform_device *pdev)
> >>> if (!ls_epc)
> >>> return -ENOMEM;
> >>>
> >>> - dbi_base = platform_get_resource_byname(pdev,
> IORESOURCE_MEM,
> >> "regs");
> >>> - pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> >>> - if (IS_ERR(pci->dbi_base))
> >>> - return PTR_ERR(pci->dbi_base);
> >>> + pcie->drvdata = of_device_get_match_data(dev);
> >>>
> >>> - pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> >>> pci->dev = dev;
> >>> - pci->ops = &ls_pcie_ep_ops;
> >>> + pci->ops = pcie->drvdata->dw_pcie_ops;
> >>> +
> >>> pcie->pci = pci;
> >>>
> >>> ls_epc->linkup_notifier = false,
> >>> @@ -152,6 +185,13 @@ static int __init ls_pcie_ep_probe(struct
> >>> platform_device *pdev)
> >>>
> >>> pcie->ls_epc = ls_epc;
> >>>
> >>> + dbi_base = platform_get_resource_byname(pdev,
> IORESOURCE_MEM,
> >> "regs");
> >>> + pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> >>> + if (IS_ERR(pci->dbi_base))
> >>> + return PTR_ERR(pci->dbi_base);
> >>> +
> >>> + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> >>> +
> >>> platform_set_drvdata(pdev, pcie);
> >>>
> >>> ret = ls_add_pcie_ep(pcie, pdev);
> >>> --
> >>> 2.9.5
> >>>
>
> ---
> L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel
> antivirus Avast.
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> avast.com%2Fantivirus&data=02%7C01%7Cxiaowei.bao%40nxp.com%7
> C1befe9a67c8046f9535e08d7285eaab6%7C686ea1d3bc2b4c6fa92cd99c5c3
> 01635%7C0%7C0%7C637022259387139020&sdata=JAYds7X%2FHVxgtrg
> e%2F%2FvnP84zdb2yReXcctQUiSLC11I%3D&reserved=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: [PATCH v2 2/2] dma-contiguous: Use fallback alloc_pages for single pages
From: Christoph Hellwig @ 2019-08-25 1:10 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Ulf Hansson, Tony Lindgren, Catalin Marinas, Will Deacon,
Linux Kernel Mailing List, Max Filippov, Christoph Hellwig,
Marek Szyprowski, Stephen Rothwell, Joerg Roedel, Russell King,
Thierry Reding, linux-xtensa, Kees Cook, Nicolin Chen,
Andrew Morton, linux-arm-kernel, Chris Zankel, Wolfram Sang,
Robin Murphy, linux-mmc, Adrian Hunter, iommu, iamjoonsoo.kim,
David Woodhouse
In-Reply-To: <CAK7LNAQfYBCoChMV=MOwcUyVoqRkrPWs7DaWdzDqjBe18gGiAQ@mail.gmail.com>
On Fri, Aug 23, 2019 at 09:56:52PM +0900, Masahiro Yamada wrote:
> + linux-mmc, Ulf Hansson, Adrian Hunter,
>
>
> ADMA of SDHCI is not working
> since bd2e75633c8012fc8a7431c82fda66237133bf7e
Does it work for you with this commit:
http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/90ae409f9eb3bcaf38688f9ec22375816053a08e
_______________________________________________
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 net-next v2 3/3] net: dsa: mt7530: Add support for port 5
From: David Miller @ 2019-08-24 23:19 UTC (permalink / raw)
To: opensource
Cc: andrew, f.fainelli, frank-w, netdev, sean.wang, linux-mips,
linux-mediatek, john, matthias.bgg, vivien.didelot,
linux-arm-kernel
In-Reply-To: <20190821144547.15113-4-opensource@vdorst.com>
From: René van Dorst <opensource@vdorst.com>
Date: Wed, 21 Aug 2019 16:45:47 +0200
> + dev_info(ds->dev, "Setup P5, HWTRAP=0x%x, intf_sel=%s, phy-mode=%s\n",
> + val, p5_intf_modes(priv->p5_intf_sel), phy_modes(interface));
This is debugging, at best. Please make this a debugging message or
remove it entirely.
_______________________________________________
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 V5 1/5] iommu/amd: Remove unnecessary locking from AMD iommu driver
From: Christoph Hellwig @ 2019-08-24 22:41 UTC (permalink / raw)
To: Tom Murphy
Cc: Heiko Stuebner, virtualization, Matthias Brugger, Thierry Reding,
Will Deacon, Jean-Philippe Brucker, linux-samsung-soc,
Krzysztof Kozlowski, Jonathan Hunter, Christoph Hellwig,
linux-rockchip, Kukjin Kim, Andy Gross, linux-s390,
Gerald Schaefer, linux-arm-msm, linux-mediatek, linux-tegra,
linux-arm-kernel, Robin Murphy, Linux Kernel Mailing List, iommu,
David Woodhouse
In-Reply-To: <CALQxJussiGDzWFT1xhko6no5jZNOezWCFuJQUCr4XwH4NHri3Q@mail.gmail.com>
Thank for the explanation Tom. It might make sense to add a condensed
version of it to commit log for the next iteration.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: cleanup the dma_pgprot handling
From: Christoph Hellwig @ 2019-08-24 22:34 UTC (permalink / raw)
To: Paul Burton
Cc: Shawn Anastasio, Will Deacon, linux-m68k@lists.linux-m68k.org,
Catalin Marinas, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, Russell King,
linux-mips@vger.kernel.org, iommu@lists.linux-foundation.org,
Geert Uytterhoeven, James Hogan, Guan Xuetao, Christoph Hellwig,
linux-arm-kernel@lists.infradead.org, Robin Murphy
In-Reply-To: <20190823215759.zprrwotlbva46y33@pburton-laptop>
On Fri, Aug 23, 2019 at 09:58:04PM +0000, Paul Burton wrote:
> So I believe uncached & uncached accelerated are another case like that
> described above - they're 2 different CCAs but the same "access type",
> namely uncached.
>
> Section 4.9 then goes on to forbid mixing access types, but not CCAs.
>
> It would be nice if the precise mapping from CCA to access type was
> provided, but I don't see that anywhere. I can check with the
> architecture team to be sure, but to my knowledge we're fine to mix
> access via kseg1 (ie. uncached) & mappings with CCA=7 (uncached
> accelerated).
Ok. Looks like we can keep it then and I'll add a comment to the
code with the above reference.
_______________________________________________
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 net-next v2 0/3] net: dsa: mt7530: Convert to PHYLINK and add support for port 5
From: Russell King - ARM Linux admin @ 2019-08-24 22:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: f.fainelli, frank-w, netdev, sean.wang, linux-mips,
vivien.didelot, opensource, linux-mediatek, john, matthias.bgg,
David Miller, linux-arm-kernel
In-Reply-To: <20190824221519.GF8251@lunn.ch>
On Sun, Aug 25, 2019 at 12:15:19AM +0200, Andrew Lunn wrote:
> 65;5402;1cOn Sat, Aug 24, 2019 at 02:18:03PM -0700, David Miller wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > Date: Fri, 23 Aug 2019 03:09:28 +0200
> >
> > > That would be Russell.
> > >
> > > We should try to improve MAINTAINER so that Russell King gets picked
> > > by the get_maintainer script.
> >
> > Shoule he be added to the mt7530 entry?
>
> Hi David
>
> No. I think we need a phylink entry. And then make use of the K: line
> format to list keywords. I hope that even though changes like this
> don't touch any files listed as being part of phylink, they will match
> the keyword and pickup Russell.
Note that phylink itself is already covered by
"SFF/SFP/SFP+ MODULE SUPPORT"
but doesn't pick up on keywords.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
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 net-next v2 0/3] net: dsa: mt7530: Convert to PHYLINK and add support for port 5
From: Russell King - ARM Linux admin @ 2019-08-24 22:29 UTC (permalink / raw)
To: René van Dorst
Cc: Andrew Lunn, Florian Fainelli, Frank Wunderlich, netdev,
Sean Wang, linux-mips, David S . Miller, linux-mediatek,
John Crispin, Matthias Brugger, Vivien Didelot, linux-arm-kernel
In-Reply-To: <20190821144547.15113-1-opensource@vdorst.com>
On Wed, Aug 21, 2019 at 04:45:44PM +0200, René van Dorst wrote:
> 1. net: dsa: mt7530: Convert to PHYLINK API
> This patch converts mt7530 to PHYLINK API.
> 2. dt-bindings: net: dsa: mt7530: Add support for port 5
> 3. net: dsa: mt7530: Add support for port 5
> These 2 patches adding support for port 5 of the switch.
>
> v1->v2:
> * Mostly phylink improvements after review.
> rfc -> v1:
> * Mostly phylink improvements after review.
> * Drop phy isolation patches. Adds no value for now.
> René van Dorst (3):
> net: dsa: mt7530: Convert to PHYLINK API
> dt-bindings: net: dsa: mt7530: Add support for port 5
> net: dsa: mt7530: Add support for port 5
>
> .../devicetree/bindings/net/dsa/mt7530.txt | 218 ++++++++++
> drivers/net/dsa/mt7530.c | 371 +++++++++++++++---
> drivers/net/dsa/mt7530.h | 61 ++-
> 3 files changed, 577 insertions(+), 73 deletions(-)
Having looked through this set of patches, I don't see anything
from the phylink point of view that concerns me. So, for the
series from the phylink perspective:
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Thanks.
I did notice a dev_info() in patch 3 that you may like to consider
whether they should be printed at info level or debug level. You
may keep my ack on the patch when fixing that.
I haven't considered whether the patch passes davem's style
requirements for networking code; what I spotted did look like
the declarations were upside-down christmas tree.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
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 net-next v2 0/3] net: dsa: mt7530: Convert to PHYLINK and add support for port 5
From: Russell King - ARM Linux admin @ 2019-08-24 22:18 UTC (permalink / raw)
To: David Miller
Cc: andrew, f.fainelli, frank-w, netdev, sean.wang, linux-mips,
opensource, linux-mediatek, john, matthias.bgg, vivien.didelot,
linux-arm-kernel
In-Reply-To: <20190824.141803.1656753287804303137.davem@davemloft.net>
On Sat, Aug 24, 2019 at 02:18:03PM -0700, David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Fri, 23 Aug 2019 03:09:28 +0200
>
> > That would be Russell.
> >
> > We should try to improve MAINTAINER so that Russell King gets picked
> > by the get_maintainer script.
>
> Shoule he be added to the mt7530 entry?
Probably some way to make MAINTAINERS pick up on phylink-containing
patches. Something like:
K: phylink
maybe?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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