* [PATCH v2 1/3] dmaengine: imx-sdma: fix buffer ownership
From: Philipp Puschmann @ 2019-09-19 10:23 UTC (permalink / raw)
To: linux-kernel
Cc: fugang.duan, Philipp Puschmann, festevam, s.hauer, vkoul,
linux-imx, kernel, dan.j.williams, yibin.gong, shawnguo,
dmaengine, linux-arm-kernel, l.stach
In-Reply-To: <20190919102319.23368-1-philipp.puschmann@emlix.com>
BD_DONE flag marks ownership of the buffer. When 1 SDMA owns the
buffer, when 0 ARM owns it. When processing the buffers in
sdma_update_channel_loop the ownership of the currently processed
buffer was set to SDMA again before running the callback function of
the buffer and while the sdma script may be running in parallel. So
there was the possibility to get the buffer overwritten by SDMA before
it has been processed by kernel leading to kind of random errors in the
upper layers, e.g. bluetooth.
Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
---
Changelog v2:
- add dma_wb()
drivers/dma/imx-sdma.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 9ba74ab7e912..e029a2443cfc 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -802,7 +802,6 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
*/
desc->chn_real_count = bd->mode.count;
- bd->mode.status |= BD_DONE;
bd->mode.count = desc->period_len;
desc->buf_ptail = desc->buf_tail;
desc->buf_tail = (desc->buf_tail + 1) % desc->num_bd;
@@ -817,6 +816,9 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
dmaengine_desc_get_callback_invoke(&desc->vd.tx, NULL);
spin_lock(&sdmac->vc.lock);
+ dma_wb();
+ bd->mode.status |= BD_DONE;
+
if (error)
sdmac->status = old_status;
}
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 2/3] dmaengine: imx-sdma: fix dma freezes
From: Philipp Puschmann @ 2019-09-19 10:23 UTC (permalink / raw)
To: linux-kernel
Cc: fugang.duan, Philipp Puschmann, festevam, s.hauer, vkoul,
linux-imx, kernel, dan.j.williams, yibin.gong, shawnguo,
dmaengine, linux-arm-kernel, l.stach
In-Reply-To: <20190919102319.23368-1-philipp.puschmann@emlix.com>
For some years and since many kernel versions there are reports that the
RX UART SDMA channel stops working at some point. The workaround was to
disable DMA for RX. This commit tries to fix the problem itself.
Due to its license i wasn't able to debug the sdma script itself but it
somehow leads to blocking the scheduling of the channel script when a
running sdma script does not find any free descriptor in the ring to put
its data into.
If we detect such a potential case we manually restart the channel.
As sdmac->desc is constant we can move desc out of the loop.
Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
Changelog v2:
- clarify comment and commit description
drivers/dma/imx-sdma.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index e029a2443cfc..a32b5962630e 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -775,21 +775,23 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
static void sdma_update_channel_loop(struct sdma_channel *sdmac)
{
struct sdma_buffer_descriptor *bd;
- int error = 0;
- enum dma_status old_status = sdmac->status;
+ struct sdma_desc *desc = sdmac->desc;
+ int error = 0, cnt = 0;
+ enum dma_status old_status = sdmac->status;
/*
* loop mode. Iterate over descriptors, re-setup them and
* call callback function.
*/
- while (sdmac->desc) {
- struct sdma_desc *desc = sdmac->desc;
+ while (desc) {
bd = &desc->bd[desc->buf_tail];
if (bd->mode.status & BD_DONE)
break;
+ cnt++;
+
if (bd->mode.status & BD_RROR) {
bd->mode.status &= ~BD_RROR;
sdmac->status = DMA_ERROR;
@@ -822,6 +824,17 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
if (error)
sdmac->status = old_status;
}
+
+ /* In some situations it may happen that the sdma does not found any
+ * usable descriptor in the ring to put data into. The channel is
+ * stopped then. While there is no specific error condition we can
+ * check for, a necessary condition is that all available buffers for
+ * the current channel have been written to by the sdma script. In
+ * this case and after we have made the buffers available again,
+ * we restart the channel.
+ */
+ if (cnt >= desc->num_bd)
+ sdma_enable_channel(sdmac->sdma, sdmac->channel);
}
static void mxc_sdma_handle_channel_normal(struct sdma_channel *data)
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 3/3] dmaengine: imx-sdma: drop redundant variable
From: Philipp Puschmann @ 2019-09-19 10:23 UTC (permalink / raw)
To: linux-kernel
Cc: fugang.duan, Philipp Puschmann, festevam, s.hauer, vkoul,
linux-imx, kernel, dan.j.williams, yibin.gong, shawnguo,
dmaengine, linux-arm-kernel, l.stach
In-Reply-To: <20190919102319.23368-1-philipp.puschmann@emlix.com>
In sdma_prep_dma_cyclic buf is redundant. Drop it.
Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
Changelog v2:
- add Reviewed-by tag
drivers/dma/imx-sdma.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index a32b5962630e..17961451941a 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1544,7 +1544,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
struct sdma_engine *sdma = sdmac->sdma;
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
- int i = 0, buf = 0;
+ int i;
struct sdma_desc *desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
@@ -1565,7 +1565,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
goto err_bd_out;
}
- while (buf < buf_len) {
+ for (i = 0; i < num_periods; i++) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
@@ -1592,9 +1592,6 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
bd->mode.status = param;
dma_addr += period_len;
- buf += period_len;
-
- i++;
}
return vchan_tx_prep(&sdmac->vc, &desc->vd, flags);
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH RFC 0/2] clk: meson: g12a: handle clock hw changes while in suspend
From: Neil Armstrong @ 2019-09-19 10:25 UTC (permalink / raw)
To: sboyd, jbrunet, mturquette
Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel,
Neil Armstrong
This serie aime to support when the suspend/resume firmware alters the
clock tree, leading to an incorrect representation of the clock tree
after a resume from suspend-to-mem.
For the Amlogic G12A/G12B/SM1 case, the SCPI firmware handling suspend
alters the CPU clock tree in various ways.
Since we know which part of the tree is possibly altered, we introduce here
the clk_invalidate_rate() function that will rebuild the tree from the
hardware registers in case parents and dividers have changed.
Finally we call clk_invalidate_rate() from a new resume callback to refresh
the CPU clock tree after a resume.
With the clock tree refreshed, CCF can now handle the new clock tree
configuration and avoid crashing the system on further DVFS set_rates.
Neil Armstrong (2):
clk: introduce clk_invalidate_rate()
clk: meson: g12a: add suspend-resume hooks
drivers/clk/clk.c | 70 +++++++++++++++++++++++++++++++++++++++
drivers/clk/meson/g12a.c | 71 +++++++++++++++++++++++++++++++++++-----
include/linux/clk.h | 13 ++++++++
3 files changed, 146 insertions(+), 8 deletions(-)
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH RFC 1/2] clk: introduce clk_invalidate_rate()
From: Neil Armstrong @ 2019-09-19 10:25 UTC (permalink / raw)
To: sboyd, jbrunet, mturquette
Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190919102518.25126-1-narmstrong@baylibre.com>
This introduces the clk_invalidate_rate() call used to recalculate the
rate and parent tree of a particular clock if it's known that the
underlying registers set has been altered by the firmware, like from
a suspend/resume handler running in trusted cpu mode.
The call refreshes the actual parent and when changed, instructs CCF
the parent has changed. Finally the call will recalculate the rate of
each part of the tree to make sure the CCF cached tree is in sync with
the hardware.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/clk/clk.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/clk.h | 13 +++++++++
2 files changed, 83 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ca99e9db6575..8acf38ce3cc4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2557,6 +2557,76 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
}
EXPORT_SYMBOL_GPL(clk_set_parent);
+/**
+ * __clk_invalidate_tree
+ * @core: first clk in the subtree
+ *
+ * Walks the subtree of clks starting with clk and recalculates the parents,
+ * then accuracies and rates as it goes.
+ */
+static int __clk_invalidate_tree(struct clk_core *core)
+{
+ struct clk_core *parent, *old_parent;
+ int ret, i, num_parents;
+
+ num_parents = core->num_parents;
+
+ for (i = 0; i < num_parents; i++) {
+ parent = clk_core_get_parent_by_index(core, i);
+ if (!parent)
+ continue;
+
+ ret = __clk_invalidate_tree(parent);
+ if (ret)
+ return ret;
+ }
+
+ parent = __clk_init_parent(core);
+
+ if (parent != core->parent) {
+ old_parent = __clk_set_parent_before(core, parent);
+ __clk_set_parent_after(core, parent, old_parent);
+ }
+
+ __clk_recalc_accuracies(core);
+ __clk_recalc_rates(core, 0);
+
+ return 0;
+}
+
+static int clk_core_invalidate_rate(struct clk_core *core)
+{
+ int ret;
+
+ clk_prepare_lock();
+
+ ret = __clk_invalidate_tree(core);
+
+ clk_prepare_unlock();
+
+ return ret;
+}
+
+/**
+ * clk_invalidate_rate - invalidate and recalc rate of the clock and it's tree
+ * @clk: the clk whose rate is too be invalidated
+ *
+ * If it's known the actual hardware state of a clock tree has changed,
+ * this call will invalidate the cached rate of the clk and it's possible
+ * parents tree to permit recalculation of the actual rate.
+ *
+ * Returns 0 on success, -EERROR otherwise.
+ * If clk is NULL then returns 0.
+ */
+int clk_invalidate_rate(struct clk *clk)
+{
+ if (!clk)
+ return 0;
+
+ return clk_core_invalidate_rate(clk->core);
+}
+EXPORT_SYMBOL_GPL(clk_invalidate_rate);
+
static int clk_core_set_phase_nolock(struct clk_core *core, int degrees)
{
int ret = -EINVAL;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 853a8f181394..46db47ffb7b2 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -629,6 +629,19 @@ long clk_round_rate(struct clk *clk, unsigned long rate);
*/
int clk_set_rate(struct clk *clk, unsigned long rate);
+/**
+ * clk_invalidate_rate - invalidate and recalc rate of the clock and it's tree
+ * @clk: the clk whose rate is too be invalidated
+ *
+ * If it's known the actual hardware state of a clock tree has changed,
+ * this call will invalidate the cached rate of the clk and it's possible
+ * parents tree to permit recalculation of the actual rate.
+ *
+ * Returns 0 on success, -EERROR otherwise.
+ * If clk is NULL then returns 0.
+ */
+int clk_invalidate_rate(struct clk *clk);
+
/**
* clk_set_rate_exclusive- set the clock rate and claim exclusivity over
* clock source
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH RFC 2/2] clk: meson: g12a: add suspend-resume hooks
From: Neil Armstrong @ 2019-09-19 10:25 UTC (permalink / raw)
To: sboyd, jbrunet, mturquette
Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190919102518.25126-1-narmstrong@baylibre.com>
Add suspend and resume hooks used to refresh the CPU clock tree
when resuming from suspend, in the case where the PSCI firmware
alters the clock tree.
In the Amlogic G12A suspend/resume case, the PSCI firmware will
alter the Fixed PLL dyn tree when entering with the CPU clock from
this same tree, but using a different path to achieve the same rate.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/clk/meson/g12a.c | 71 +++++++++++++++++++++++++++++++++++-----
1 file changed, 63 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index b3af61cc6fb9..9f6f634225b7 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -4992,6 +4992,19 @@ static int meson_g12b_dvfs_setup(struct platform_device *pdev)
return 0;
}
+static int meson_g12b_resume(struct device *dev)
+{
+ u32 ret;
+
+ ret = clk_invalidate_rate(
+ __clk_lookup(clk_hw_get_name(&g12b_cpu_clk.hw)));
+ if (ret)
+ return ret;
+
+ return clk_invalidate_rate(
+ __clk_lookup(clk_hw_get_name(&g12b_cpub_clk.hw)));
+}
+
static int meson_g12a_dvfs_setup(struct platform_device *pdev)
{
struct clk_hw **hws = g12a_hw_onecell_data.hws;
@@ -5024,34 +5037,68 @@ static int meson_g12a_dvfs_setup(struct platform_device *pdev)
return 0;
}
+static int meson_g12a_resume(struct device *dev)
+{
+ return clk_invalidate_rate(
+ __clk_lookup(clk_hw_get_name(&g12a_cpu_clk.hw)));
+}
+
struct meson_g12a_data {
const struct meson_eeclkc_data eeclkc_data;
int (*dvfs_setup)(struct platform_device *pdev);
+ int (*resume)(struct device *dev);
};
+static const struct
+meson_g12a_data *meson_g12a_get_data(struct device *dev)
+{
+ const struct meson_eeclkc_data *eeclkc_data =
+ of_device_get_match_data(dev);
+
+ if (!eeclkc_data)
+ return ERR_PTR(-EINVAL);
+
+ return container_of(eeclkc_data, struct meson_g12a_data,
+ eeclkc_data);
+}
+
static int meson_g12a_probe(struct platform_device *pdev)
{
- const struct meson_eeclkc_data *eeclkc_data;
- const struct meson_g12a_data *g12a_data;
int ret;
+ const struct meson_g12a_data *g12a_data =
+ meson_g12a_get_data(&pdev->dev);
- eeclkc_data = of_device_get_match_data(&pdev->dev);
- if (!eeclkc_data)
- return -EINVAL;
+ if (IS_ERR(g12a_data))
+ return PTR_ERR(g12a_data);
ret = meson_eeclkc_probe(pdev);
if (ret)
return ret;
- g12a_data = container_of(eeclkc_data, struct meson_g12a_data,
- eeclkc_data);
-
if (g12a_data->dvfs_setup)
return g12a_data->dvfs_setup(pdev);
return 0;
}
+static int __maybe_unused g12a_clkc_suspend(struct device *dev)
+{
+ return 0;
+}
+
+static int __maybe_unused g12a_clkc_resume(struct device *dev)
+{
+ const struct meson_g12a_data *g12a_data = meson_g12a_get_data(dev);
+
+ if (IS_ERR(g12a_data))
+ return PTR_ERR(g12a_data);
+
+ if (g12a_data->resume)
+ return g12a_data->resume(dev);
+
+ return 0;
+}
+
static const struct meson_g12a_data g12a_clkc_data = {
.eeclkc_data = {
.regmap_clks = g12a_clk_regmaps,
@@ -5061,6 +5108,7 @@ static const struct meson_g12a_data g12a_clkc_data = {
.init_count = ARRAY_SIZE(g12a_init_regs),
},
.dvfs_setup = meson_g12a_dvfs_setup,
+ .resume = meson_g12a_resume,
};
static const struct meson_g12a_data g12b_clkc_data = {
@@ -5070,6 +5118,7 @@ static const struct meson_g12a_data g12b_clkc_data = {
.hw_onecell_data = &g12b_hw_onecell_data,
},
.dvfs_setup = meson_g12b_dvfs_setup,
+ .resume = meson_g12b_resume,
};
static const struct meson_g12a_data sm1_clkc_data = {
@@ -5079,6 +5128,11 @@ static const struct meson_g12a_data sm1_clkc_data = {
.hw_onecell_data = &sm1_hw_onecell_data,
},
.dvfs_setup = meson_g12a_dvfs_setup,
+ .resume = meson_g12a_resume,
+};
+
+static const struct dev_pm_ops g12a_clkc_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(g12a_clkc_suspend, g12a_clkc_resume)
};
static const struct of_device_id clkc_match_table[] = {
@@ -5102,6 +5156,7 @@ static struct platform_driver g12a_driver = {
.driver = {
.name = "g12a-clkc",
.of_match_table = clkc_match_table,
+ .pm = &g12a_clkc_dev_pm_ops,
},
};
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2] serial: imx: adapt rx buffer and dma periods
From: Philipp Puschmann @ 2019-09-19 10:26 UTC (permalink / raw)
To: linux-kernel
Cc: festevam, fugang.duan, linux-serial, gregkh, s.hauer, linux-imx,
kernel, jslaby, Philipp Puschmann, yibin.gong, shawnguo,
linux-arm-kernel, l.stach
Using only 4 DMA periods for UART RX is very few if we have a high
frequency of small transfers - like in our case using Bluetooth with
many small packets via UART - causing many dma transfers but in each
only filling a fraction of a single buffer. Such a case may lead to
the situation that DMA RX transfer is triggered but no free buffer is
available. While we have addressed the dma handling already with
"dmaengine: imx-sdma: fix dma freezes" we still want to avoid
UART RX FIFO overrun. So we decrease the size of the buffers and
increase their number and the total buffer size.
Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
Changelog v2:
- split this patch from series "Fix UART DMA freezes for iMX6"
- add Reviewed-by tag
drivers/tty/serial/imx.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 87c58f9f6390..51dc19833eab 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1034,8 +1034,6 @@ static void imx_uart_timeout(struct timer_list *t)
}
}
-#define RX_BUF_SIZE (PAGE_SIZE)
-
/*
* There are two kinds of RX DMA interrupts(such as in the MX6Q):
* [1] the RX DMA buffer is full.
@@ -1118,7 +1116,8 @@ static void imx_uart_dma_rx_callback(void *data)
}
/* RX DMA buffer periods */
-#define RX_DMA_PERIODS 4
+#define RX_DMA_PERIODS 16
+#define RX_BUF_SIZE (PAGE_SIZE / 4)
static int imx_uart_start_rx_dma(struct imx_port *sport)
{
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v2 1/3] dmaengine: imx-sdma: fix buffer ownership
From: Lucas Stach @ 2019-09-19 10:27 UTC (permalink / raw)
To: Philipp Puschmann, linux-kernel
Cc: fugang.duan, festevam, s.hauer, vkoul, linux-imx, kernel,
dmaengine, dan.j.williams, yibin.gong, shawnguo, linux-arm-kernel
In-Reply-To: <20190919102319.23368-2-philipp.puschmann@emlix.com>
Hi Philipp,
On Do, 2019-09-19 at 12:23 +0200, Philipp Puschmann wrote:
> BD_DONE flag marks ownership of the buffer. When 1 SDMA owns the
> buffer, when 0 ARM owns it. When processing the buffers in
> sdma_update_channel_loop the ownership of the currently processed
> buffer was set to SDMA again before running the callback function of
> the buffer and while the sdma script may be running in parallel. So
> there was the possibility to get the buffer overwritten by SDMA
> before
> it has been processed by kernel leading to kind of random errors in
> the
> upper layers, e.g. bluetooth.
>
> Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
>
> ---
>
> Changelog v2:
> - add dma_wb()
>
> drivers/dma/imx-sdma.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 9ba74ab7e912..e029a2443cfc 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -802,7 +802,6 @@ static void sdma_update_channel_loop(struct
> sdma_channel *sdmac)
> */
>
> desc->chn_real_count = bd->mode.count;
> - bd->mode.status |= BD_DONE;
> bd->mode.count = desc->period_len;
> desc->buf_ptail = desc->buf_tail;
> desc->buf_tail = (desc->buf_tail + 1) % desc->num_bd;
> @@ -817,6 +816,9 @@ static void sdma_update_channel_loop(struct
> sdma_channel *sdmac)
> dmaengine_desc_get_callback_invoke(&desc->vd.tx, NULL);
> spin_lock(&sdmac->vc.lock);
>
> + dma_wb();
Has this change been tested? The function you want here is called
dma_wmb().
Regards,
Lucas
> + bd->mode.status |= BD_DONE;
> +
> if (error)
> sdmac->status = old_status;
> }
_______________________________________________
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 0/3] Fix UART DMA freezes for i.MX SOCs
From: Fabio Estevam @ 2019-09-19 10:33 UTC (permalink / raw)
To: Philipp Puschmann
Cc: Fugang Duan, Sascha Hauer, linux-kernel, Vinod, NXP Linux Team,
Sascha Hauer, Dan Williams, Robin Gong, Shawn Guo, dmaengine,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Lucas Stach
In-Reply-To: <20190919102319.23368-1-philipp.puschmann@emlix.com>
Hi Philipp,
On Thu, Sep 19, 2019 at 7:23 AM Philipp Puschmann
<philipp.puschmann@emlix.com> wrote:
> Philipp Puschmann (3):
> dmaengine: imx-sdma: fix buffer ownership
> dmaengine: imx-sdma: fix dma freezes
These two fixes deserve a Fixes tag so that they could be backported
to the stable tree.
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 v2 1/3] dmaengine: imx-sdma: fix buffer ownership
From: Philipp Puschmann @ 2019-09-19 10:34 UTC (permalink / raw)
To: Lucas Stach, linux-kernel
Cc: fugang.duan, festevam, s.hauer, vkoul, linux-imx, kernel,
dmaengine, dan.j.williams, yibin.gong, shawnguo, linux-arm-kernel
In-Reply-To: <7d694da8ffe098c6c8f6fe9c3a2306fda55eb655.camel@pengutronix.de>
Hi Lucas,
Am 19.09.19 um 12:27 schrieb Lucas Stach:
> Hi Philipp,
>
> On Do, 2019-09-19 at 12:23 +0200, Philipp Puschmann wrote:
>> BD_DONE flag marks ownership of the buffer. When 1 SDMA owns the
>> buffer, when 0 ARM owns it. When processing the buffers in
>> sdma_update_channel_loop the ownership of the currently processed
>> buffer was set to SDMA again before running the callback function of
>> the buffer and while the sdma script may be running in parallel. So
>> there was the possibility to get the buffer overwritten by SDMA
>> before
>> it has been processed by kernel leading to kind of random errors in
>> the
>> upper layers, e.g. bluetooth.
>>
>> Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
>>
>> ---
>>
>> Changelog v2:
>> - add dma_wb()
>>
>> drivers/dma/imx-sdma.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
>> index 9ba74ab7e912..e029a2443cfc 100644
>> --- a/drivers/dma/imx-sdma.c
>> +++ b/drivers/dma/imx-sdma.c
>> @@ -802,7 +802,6 @@ static void sdma_update_channel_loop(struct
>> sdma_channel *sdmac)
>> */
>>
>> desc->chn_real_count = bd->mode.count;
>> - bd->mode.status |= BD_DONE;
>> bd->mode.count = desc->period_len;
>> desc->buf_ptail = desc->buf_tail;
>> desc->buf_tail = (desc->buf_tail + 1) % desc->num_bd;
>> @@ -817,6 +816,9 @@ static void sdma_update_channel_loop(struct
>> sdma_channel *sdmac)
>> dmaengine_desc_get_callback_invoke(&desc->vd.tx, NULL);
>> spin_lock(&sdmac->vc.lock);
>>
>> + dma_wb();
>
> Has this change been tested? The function you want here is called
> dma_wmb().
embarrassingly you are right. c&p error and even have not tried to build it :/
V3 comes soon..
Regards,
Philipp
>
> Regards,
> Lucas
>
>> + bd->mode.status |= BD_DONE;
>> +
>> if (error)
>> sdmac->status = old_status;
>> }
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 1/3] dmaengine: imx-sdma: fix buffer ownership
From: Philipp Puschmann @ 2019-09-19 10:45 UTC (permalink / raw)
To: linux-kernel
Cc: fugang.duan, Philipp Puschmann, festevam, s.hauer, vkoul,
linux-imx, kernel, dan.j.williams, yibin.gong, shawnguo,
dmaengine, linux-arm-kernel, l.stach
In-Reply-To: <20190919104526.29851-1-philipp.puschmann@emlix.com>
BD_DONE flag marks ownership of the buffer. When 1 SDMA owns the
buffer, when 0 ARM owns it. When processing the buffers in
sdma_update_channel_loop the ownership of the currently processed
buffer was set to SDMA again before running the callback function of
the buffer and while the sdma script may be running in parallel. So
there was the possibility to get the buffer overwritten by SDMA before
it has been processed by kernel leading to kind of random errors in the
upper layers, e.g. bluetooth.
Fixes: broken since start
Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
---
Changelog v3:
- use correct dma_wmb() instead of dma_wb()
- add fixes tag
Changelog v2:
- add dma_wb()
drivers/dma/imx-sdma.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 9ba74ab7e912..e029a2443cfc 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -802,7 +802,6 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
*/
desc->chn_real_count = bd->mode.count;
- bd->mode.status |= BD_DONE;
bd->mode.count = desc->period_len;
desc->buf_ptail = desc->buf_tail;
desc->buf_tail = (desc->buf_tail + 1) % desc->num_bd;
@@ -817,6 +816,9 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
dmaengine_desc_get_callback_invoke(&desc->vd.tx, NULL);
spin_lock(&sdmac->vc.lock);
+ dma_wmb();
+ bd->mode.status |= BD_DONE;
+
if (error)
sdmac->status = old_status;
}
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 0/3] Fix UART DMA freezes for i.MX SOCs
From: Philipp Puschmann @ 2019-09-19 10:45 UTC (permalink / raw)
To: linux-kernel
Cc: fugang.duan, Philipp Puschmann, festevam, s.hauer, vkoul,
linux-imx, kernel, dan.j.williams, yibin.gong, shawnguo,
dmaengine, linux-arm-kernel, l.stach
In-Reply-To: <20190919102319.23368-1-philipp.puschmann@emlix.com>
For some years and since many kernel versions there are reports that
RX UART DMA channel stops working at one point. So far the usual
workaround was to disable RX DMA. This patches fix the underlying
problem.
When a running sdma script does not find any usable destination buffer
to put its data into it just leads to stopping the channel being
scheduled again. As solution we manually retrigger the sdma script for
this channel and by this dissolve the freeze.
While this seems to work fine so far, it may come to buffer overruns
when the channel - even temporary - is stopped. This case has to be
addressed by device drivers by increasing the number of DMA periods.
This patch series was tested with the current kernel and backported to
kernel 4.15 with a special use case using a WL1837MOD via UART and
provoking the hanging of UART RX DMA within seconds after starting a
test application. It resulted in well known
"Bluetooth: hci0: command 0x0408 tx timeout"
errors and complete stop of UART data reception. Our Bluetooth traffic
consists of many independent small packets, mostly only a few bytes,
causing high usage of periods.
Changelog v3:
- fixes typo in dma_wmb
- add fixes tags
Changelog v2:
- adapt title (this patches are not only for i.MX6)
- improve some comments and patch descriptions
- add a dma_wb() around BD_DONE flag
- add Reviewed-by tags
- split off "serial: imx: adapt rx buffer and dma periods"
Philipp Puschmann (3):
dmaengine: imx-sdma: fix buffer ownership
dmaengine: imx-sdma: fix dma freezes
dmaengine: imx-sdma: drop redundant variable
drivers/dma/imx-sdma.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 3/3] dmaengine: imx-sdma: drop redundant variable
From: Philipp Puschmann @ 2019-09-19 10:45 UTC (permalink / raw)
To: linux-kernel
Cc: fugang.duan, Philipp Puschmann, festevam, s.hauer, vkoul,
linux-imx, kernel, dan.j.williams, yibin.gong, shawnguo,
dmaengine, linux-arm-kernel, l.stach
In-Reply-To: <20190919104526.29851-1-philipp.puschmann@emlix.com>
In sdma_prep_dma_cyclic buf is redundant. Drop it.
Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
Changelog v3:
- no changes
Changelog v2:
- add Reviewed-by tag
drivers/dma/imx-sdma.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index a32b5962630e..17961451941a 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1544,7 +1544,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
struct sdma_engine *sdma = sdmac->sdma;
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
- int i = 0, buf = 0;
+ int i;
struct sdma_desc *desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
@@ -1565,7 +1565,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
goto err_bd_out;
}
- while (buf < buf_len) {
+ for (i = 0; i < num_periods; i++) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
@@ -1592,9 +1592,6 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
bd->mode.status = param;
dma_addr += period_len;
- buf += period_len;
-
- i++;
}
return vchan_tx_prep(&sdmac->vc, &desc->vd, flags);
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 2/3] dmaengine: imx-sdma: fix dma freezes
From: Philipp Puschmann @ 2019-09-19 10:45 UTC (permalink / raw)
To: linux-kernel
Cc: fugang.duan, Philipp Puschmann, festevam, s.hauer, vkoul,
linux-imx, kernel, dan.j.williams, yibin.gong, shawnguo,
dmaengine, linux-arm-kernel, l.stach
In-Reply-To: <20190919104526.29851-1-philipp.puschmann@emlix.com>
For some years and since many kernel versions there are reports that the
RX UART SDMA channel stops working at some point. The workaround was to
disable DMA for RX. This commit tries to fix the problem itself.
Due to its license i wasn't able to debug the sdma script itself but it
somehow leads to blocking the scheduling of the channel script when a
running sdma script does not find any free descriptor in the ring to put
its data into.
If we detect such a potential case we manually restart the channel.
As sdmac->desc is constant we can move desc out of the loop.
Fixes: broken UART RX DMA. broken since start
Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
Changelog v3:
- use correct dma_wmb() instead of dma_wb()
- add fixes tag
Changelog v2:
- clarify comment and commit description
drivers/dma/imx-sdma.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index e029a2443cfc..a32b5962630e 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -775,21 +775,23 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
static void sdma_update_channel_loop(struct sdma_channel *sdmac)
{
struct sdma_buffer_descriptor *bd;
- int error = 0;
- enum dma_status old_status = sdmac->status;
+ struct sdma_desc *desc = sdmac->desc;
+ int error = 0, cnt = 0;
+ enum dma_status old_status = sdmac->status;
/*
* loop mode. Iterate over descriptors, re-setup them and
* call callback function.
*/
- while (sdmac->desc) {
- struct sdma_desc *desc = sdmac->desc;
+ while (desc) {
bd = &desc->bd[desc->buf_tail];
if (bd->mode.status & BD_DONE)
break;
+ cnt++;
+
if (bd->mode.status & BD_RROR) {
bd->mode.status &= ~BD_RROR;
sdmac->status = DMA_ERROR;
@@ -822,6 +824,17 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
if (error)
sdmac->status = old_status;
}
+
+ /* In some situations it may happen that the sdma does not found any
+ * usable descriptor in the ring to put data into. The channel is
+ * stopped then. While there is no specific error condition we can
+ * check for, a necessary condition is that all available buffers for
+ * the current channel have been written to by the sdma script. In
+ * this case and after we have made the buffers available again,
+ * we restart the channel.
+ */
+ if (cnt >= desc->num_bd)
+ sdma_enable_channel(sdmac->sdma, sdmac->channel);
}
static void mxc_sdma_handle_channel_normal(struct sdma_channel *data)
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
From: Paolo Bonzini @ 2019-09-19 11:07 UTC (permalink / raw)
To: Jianyong Wu (Arm Technology China), netdev@vger.kernel.org,
yangbo.lu@nxp.com, john.stultz@linaro.org, tglx@linutronix.de,
sean.j.christopherson@intel.com, maz@kernel.org,
richardcochran@gmail.com, Mark Rutland, Will Deacon,
Suzuki Poulose
Cc: Justin He (Arm Technology China), kvm@vger.kernel.org,
Steve Capper, linux-kernel@vger.kernel.org,
Kaly Xin (Arm Technology China), nd,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <HE1PR0801MB167639E2F025998058A77F86F4890@HE1PR0801MB1676.eurprd08.prod.outlook.com>
On 19/09/19 11:46, Jianyong Wu (Arm Technology China) wrote:
>> On 18/09/19 11:57, Jianyong Wu (Arm Technology China) wrote:
>>> Paolo Bonzini wrote:
>>>> This is not Y2038-safe. Please use ktime_get_real_ts64 instead, and
>>>> split the 64-bit seconds value between val[0] and val[1].
>
> Val[] should be long not u32 I think, so in arm64 I can avoid that Y2038_safe, but
> also need rewrite for arm32.
I don't think there's anything inherently wrong with u32 val[], and as
you notice it lets you reuse code between arm and arm64. It's up to you
and Marc to decide.
>>>> However, it seems to me that the new function is not needed and you
>>>> can just use ktime_get_snapshot. You'll get the time in
>>>> systime_snapshot->real and the cycles value in systime_snapshot->cycles.
>>>
>>> See patch 5/6, I need both counter cycle and clocksource,
>> ktime_get_snapshot seems only offer cycles.
>>
>> No, patch 5/6 only needs the current clock (ptp_sc.cycles is never accessed).
>> So you could just use READ_ONCE(tk->tkr_mono.clock).
>>
> Yeah, patch 5/6 just need clocksource, but I think tk->tkr_mono.clock can't read in external like module,
> So I need an API to expose clocksource.
>
>> However, even then I don't think it is correct to use ptp_sc.cs blindly in patch
>> 5. I think there is a misunderstanding on the meaning of
>> system_counterval.cs as passed to get_device_system_crosststamp.
>> system_counterval.cs is not the active clocksource; it's the clocksource on
>> which system_counterval.cycles is based.
>>
>
> I think we can use system_counterval_t as pass current clocksource to system_counterval_t.cs and its
> corresponding cycles to system_counterval_t.cycles. is it a big problem?
Yes, it is. Because...
>> Hypothetically, the clocksource could be one for which ptp_sc.cycles is _not_
>> a cycle value. If you set system_counterval.cs to the system clocksource,
>> get_device_system_crosststamp will return a bogus value.
>
> Yeah, but in patch 3/6, we have a corresponding pair of clock source and cycle value. So I think there will be no
> that problem in this patch set.
> In the implementation of get_device_system_crosststamp:
> "
> ...
> if (tk->tkr_mono.clock != system_counterval.cs)
> return -ENODEV;
> ...
> "
> We need tk->tkr_mono.clock passed to get_device_system_crosststamp, just like patch 3/6 do, otherwise will return error.
... if the hypercall returns an architectural timer value, you must not
pass tk->tkr.mono.clock to get_device_system_crosststamp: you must pass
&clocksource_counter. This way, PTP is disabled when using any other
clocksource.
>> So system_counterval.cs should be set to something like
>> &clocksource_counter (from drivers/clocksource/arm_arch_timer.c).
>> Perhaps the right place to define kvm_arch_ptp_get_clock_fn is in that file?
>>
> I have checked that ptp_sc.cs is arch_sys_counter.
> Also move the module API to arm_arch_timer.c will looks a little
> ugly and it's not easy to be accept by arm side I think.
I don't think it's ugly but more important, using tk->tkr_mono.clock is
incorrect. See how the x86 code hardcodes &kvm_clock, it's the same for
ARM.
>> You also have to check here that the clocksource is based on the ARM
>> architectural timer. Again, maybe you could place the implementation in
>> drivers/clocksource/arm_arch_timer.c, and make it return -ENODEV if the
>> active clocksource is not clocksource_counter. Then KVM can look for errors
>> and return SMCCC_RET_NOT_SUPPORTED in that case.
>
> I have checked it. The clock source is arch_sys_counter which is arm arch timer.
> I can try to do that but I'm not sure arm side will be happy for that change.
Just try. For my taste, it's nice to include both sides of the
hypercall in drivers/clocksource/arm_arch_timer.c, possibly
conditionalizing them on #ifdef CONFIG_KVM and #ifdef
CONFIG_PTP_1588_CLOCK_KVM. But there is an alternative which is simply
to export the clocksource struct. Both choices are easy to implement so
you can just ask the ARM people what they prefer and they can judge from
the code.
Paolo
_______________________________________________
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] pwm: rockchip: simplify rockchip_pwm_get_state()
From: oUwe Kleine-König @ 2019-09-19 11:11 UTC (permalink / raw)
To: Rasmus Villemoes, David Wu
Cc: linux-pwm, Heiko Stuebner, linux-kernel, linux-rockchip,
Thierry Reding, linux-arm-kernel
In-Reply-To: <20190919091728.24756-1-linux@rasmusvillemoes.dk>
On Thu, Sep 19, 2019 at 11:17:27AM +0200, Rasmus Villemoes wrote:
> The way state->enabled is computed is rather convoluted and hard to
> read - both branches of the if() actually do the exact same thing. So
> remove the if(), and further simplify "<boolean condition> ? true :
> false" to "<boolean condition>".
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> I stumbled on this while trying to understand how the pwm subsystem
> works. This patch is a semantic no-op, but it's also possible that,
> say, the first branch simply contains a "double negative" so either
> the != should be == or the "false : true" should be "true : false".
The change looks obviously right, it's a noop.
I share your doubts however. The construct was introduced in commit
831b2790507b ("pwm: rockchip: Use same PWM ops for each IP") by David
Wu.
Before there were rockchip_pwm_get_state_v1 for the supports_polarity =
false case and rockchip_pwm_get_state_v2 for supports_polarity = true.
In both state->enabled was assigned true if ((val & enable_conf) ==
enable_conf). So I assume everything is fine.
A confirmation by David would be great though.
As a side note: Is there publicly available documentation for this IP?
If a link were added to the driver's header we could check easily
ourselves.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
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 1/1] sched/rt: avoid contend with CFS task
From: Jing-Ting Wu @ 2019-09-19 11:22 UTC (permalink / raw)
To: Vincent Guittot
Cc: wsd_upstream, Peter Zijlstra, linux-kernel, Valentin Schneider,
linux-mediatek, Matthias Brugger, Qais Yousef, LAK
In-Reply-To: <CAKfTPtC3txstND=6YkWBJ16i06cQ7xueUpD5j-j-UfuSf0-z-g@mail.gmail.com>
On Thu, 2019-09-05 at 16:01 +0200, Vincent Guittot wrote:
> Hi Jing-Ting,
>
> On Thu, 5 Sep 2019 at 15:26, Jing-Ting Wu <jing-ting.wu@mediatek.com> wrote:
> >
> > On Fri, 2019-08-30 at 15:55 +0100, Qais Yousef wrote:
> > > On 08/29/19 11:38, Valentin Schneider wrote:
> > > > On 29/08/2019 04:15, Jing-Ting Wu wrote:
> > > > > At original linux design, RT & CFS scheduler are independent.
> > > > > Current RT task placement policy will select the first cpu in
> > > > > lowest_mask, even if the first CPU is running a CFS task.
> > > > > This may put RT task to a running cpu and let CFS task runnable.
> > > > >
> > > > > So we select idle cpu in lowest_mask first to avoid preempting
> > > > > CFS task.
> > > > >
> > > >
> > > > Regarding the RT & CFS thing, that's working as intended. RT is a whole
> > > > class above CFS, it shouldn't have to worry about CFS.
> > > >
> > > > On the other side of things, CFS does worry about RT. We have the concept
> > > > of RT-pressure in the CFS scheduler, where RT tasks will reduce a CPU's
> > > > capacity (see fair.c::scale_rt_capacity()).
> > > >
> > > > CPU capacity is looked at on CFS wakeup (see wake_cap() and
> > > > find_idlest_cpu()), and the periodic load balancer tries to spread load
> > > > over capacity, so it'll tend to put less things on CPUs that are also
> > > > running RT tasks.
> > > >
> > > > If RT were to start avoiding rqs with CFS tasks, we'd end up with a nasty
> > > > situation were both are avoiding each other. It's even more striking when
> > > > you see that RT pressure is done with a rq-wide RT util_avg, which
> > > > *doesn't* get migrated when a RT task migrates. So if you decide to move
> > > > a RT task to an idle CPU "B" because CPU "A" had runnable CFS tasks, the
> > > > CFS scheduler will keep seeing CPU "B" as not significantly RT-pressured
> > > > while that util_avg signal ramps up, whereas it would correctly see CPU
> > > > "A" as RT-pressured if the RT task previously ran there.
> > > >
> > > > So overall I think this is the wrong approach.
> > >
> > > I like the idea, but yeah tend to agree the current approach might not be
> > > enough.
> > >
> > > I think the major problem here is that on generic systems where CFS is a first
> > > class citizen, RT tasks can be hostile to them - not always necessarily for a
> > > good reason.
> > >
> > > To further complicate the matter, even among CFS tasks we can't tell which are
> > > more important than the others - though hopefully latency-nice proposal will
> > > make the situation better.
> > >
> > > So I agree we have a problem here, but I think this patch is just a temporary
> > > band aid and we need to do better. Though I have no concrete suggestion yet on
> > > how to do that.
> > >
> > > Another thing I couldn't quantify yet how common and how severe this problem is
> > > yet. Jing-Ting, if you can share the details of your use case that'd be great.
> > >
> > > Cheers
> > >
> > > --
> > > Qais Yousef
> >
> >
> > I agree that the nasty situation will happen.The current approach and this patch might not be enough.
>
> RT task should not harm its cache hotness and responsiveness for the
> benefit of a CFS task
>
Yes, it’s a good point to both consider cache hotness. We have revised
the implementation to select a better idle CPU in the same sched_domain
of prev_cpu (with the same cache hotness) when the RT task wakeup.
I modify the code of find_lowest_rq as following:
@@ -1648,6 +1629,9 @@ static int find_lowest_rq(struct task_struct
*task)
struct cpumask *lowest_mask =
this_cpu_cpumask_var_ptr(local_cpu_mask);
int this_cpu = smp_processor_id();
int cpu = task_cpu(task);
+ int i;
+ struct rq *prev_rq = cpu_rq(cpu);
+ struct sched_domain *prev_sd;
/* Make sure the mask is initialized first */
if (unlikely(!lowest_mask))
@@ -1659,6 +1643,24 @@ static int find_lowest_rq(struct task_struct
*task)
if (!cpupri_find(&task_rq(task)->rd->cpupri, task, lowest_mask))
return -1; /* No targets found */
+ /* Choose previous cpu if it is idle and it fits lowest_mask */
+ if (cpumask_test_cpu(cpu, lowest_mask) && idle_cpu(cpu))
+ return cpu;
+
+ rcu_read_lock();
+ prev_sd = rcu_dereference(prev_rq->sd);
+
+ if (prev_sd) {
+ /* Choose idle_cpu among lowest_mask and it is closest
to our hot cache data */
+ for_each_cpu(i, lowest_mask) {
+ if (idle_cpu(i) && cpumask_test_cpu(i,
sched_domain_span(prev_sd))) {
+ rcu_read_unlock();
+ return i;
+ }
+ }
+ }
+ rcu_read_unlock();
+
/*
* At this point we have built a mask of CPUs representing the
* lowest priority tasks in the system. Now we want to elect
> > But for requirement of performance, I think it is better to differentiate between idle CPU and CPU has CFS task.
> >
> > For example, we use rt-app to evaluate runnable time on non-patched environment.
> > There are (NR_CPUS-1) heavy CFS tasks and 1 RT Task. When a CFS task is running, the RT task wakes up and choose the same CPU.
> > The CFS task will be preempted and keep runnable until it is migrated to another cpu by load balance.
> > But load balance is not triggered immediately, it will be triggered until timer tick hits with some condition satisfied(ex. rq->next_balance).
>
> Yes you will have to wait for the next tick that will trigger an idle
> load balance because you have an idle cpu and 2 runnable tack (1 RT +
> 1CFS) on the same CPU. But you should not wait for more than 1 tick
>
> The current load_balance doesn't handle correctly the situation of 1
> CFS and 1 RT task on same CPU while 1 CPU is idle. There is a rework
> of the load_balance that is under review on the mailing list that
> fixes this problem and your CFS task should migrate to the idle CPU
> faster than now
>
Period load balance should be triggered when current jiffies is behind
rq->next_balance, but rq->next_balance is not often exactly the same
with next tick.
If cpu_busy, interval = sd->balance_interval * sd->busy_factor, and
interval is clamped by 1 to max_load_balance_interval.
By experiment, in a system with HZ=250, available_cpus = 8, the
max_load_balance_interval = HZ * available_cpus / 10 = 250 * 8 / 10 =
200 jiffies,
It would let rq->next_balance = sd->last_balance + interval, the maximum
interval = 200 jiffies, result in more than 1 sched-tick to migrate a
CFS task.
> > CFS tasks may be runnable for a long time. In this test case, it increase 332.091 ms runnable time for CFS task.
> >
> > The detailed log is shown as following, CFS task(thread1-6580) is preempted by RT task(thread0-6674) about 332ms:
>
> 332ms is quite long and is probably not an idle load blanace but a
> busy load balance
>
> > thread1-6580 [003] dnh2 94.452898: sched_wakeup: comm=thread0 pid=6674 prio=89 target_cpu=003
> > thread1-6580 [003] d..2 94.452916: sched_switch: prev_comm=thread1 prev_pid=6580 prev_prio=120 prev_state=R ==> next_comm=thread0 next_pid=6674 next_prio=89
> > .... 332.091ms
> > krtatm-1930 [001] d..2 94.785007: sched_migrate_task: comm=thread1 pid=6580 prio=120 orig_cpu=3 dest_cpu=1
> > krtatm-1930 [001] d..2 94.785020: sched_switch: prev_comm=krtatm prev_pid=1930 prev_prio=100 prev_state=S ==> next_comm=thread1 next_pid=6580 next_prio=120
>
> your CFS task has not moved on the idle CPU but has replaced another task
>
I think it is minor and reasonable, because CPU1 has triggered idle
balance (when krtatm task is the last task leaving CPU1) to pull the
thread1-6580.
Best regards,
Jing-Ting Wu
> Regards,
> Vincent
> >
> > So I think choose idle CPU at RT wake up flow could reduce the CFS runnable time.
> >
> >
> > Best regards,
> > Jing-Ting Wu
> >
> >
_______________________________________________
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] serial: imx: adapt rx buffer and dma periods
From: Uwe Kleine-König @ 2019-09-19 11:22 UTC (permalink / raw)
To: Philipp Puschmann
Cc: festevam, fugang.duan, linux-serial, gregkh, s.hauer,
linux-kernel, linux-imx, kernel, jslaby, yibin.gong, shawnguo,
linux-arm-kernel, l.stach
In-Reply-To: <20190919102628.23621-1-philipp.puschmann@emlix.com>
On Thu, Sep 19, 2019 at 12:26:28PM +0200, Philipp Puschmann wrote:
> Using only 4 DMA periods for UART RX is very few if we have a high
> frequency of small transfers - like in our case using Bluetooth with
> many small packets via UART - causing many dma transfers but in each
> only filling a fraction of a single buffer. Such a case may lead to
> the situation that DMA RX transfer is triggered but no free buffer is
> available. While we have addressed the dma handling already with
> "dmaengine: imx-sdma: fix dma freezes" we still want to avoid
Is this statement still true now that you split this patch out of your
bigger series?
> UART RX FIFO overrun. So we decrease the size of the buffers and
> increase their number and the total buffer size.
What happens when such an RX FIFO overrun happens? Are characters lost?
Or only time? Does your change have an influence if I do fewer but
bigger transfers?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
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] usb: dwc3: Add shutdown to platform_driver
From: Vicente Bergas @ 2019-09-19 11:36 UTC (permalink / raw)
To: Felipe Balbi
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
Robin Murphy, linux-arm-kernel
In-Reply-To: <20190817174140.6394-1-vicencb@gmail.com>
Ping?
On Saturday, August 17, 2019 7:41:40 PM CEST, Vicente Bergas wrote:
> Otherwise the device keeps writing to memory after kexec and disturbs
> the next kernel.
>
> Signed-off-by: Vicente Bergas <vicencb@gmail.com>
> ---
> drivers/usb/dwc3/dwc3-of-simple.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> Hi Felipe, Robin,
> this version calls 'remove' from 'shutdown' instead of just asserting
> a reset because it looks like a cleaner way to stop the device.
>
> Calling remove from shutdown in core.c instead of dwc3-of-simple.c does not
> fix the issue either.
>
> It has been tested on the sapphire board, a RK3399 platform.
>
> Regards,
> Vicenç.
>
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c
> b/drivers/usb/dwc3/dwc3-of-simple.c
> index bdac3e7d7b18..d5fd45c64901 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -133,6 +133,11 @@ static int dwc3_of_simple_remove(struct
> platform_device *pdev)
> return 0;
> }
>
> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
> +{
> + dwc3_of_simple_remove(pdev);
> +}
> +
> static int __maybe_unused
> dwc3_of_simple_runtime_suspend(struct device *dev)
> {
> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
> @@ -190,6 +195,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
> static struct platform_driver dwc3_of_simple_driver = {
> .probe = dwc3_of_simple_probe,
> .remove = dwc3_of_simple_remove,
> + .shutdown = dwc3_of_simple_shutdown,
> .driver = {
> .name = "dwc3-of-simple",
> .of_match_table = of_dwc3_simple_match,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/3] dmaengine: imx-sdma: fix buffer ownership
From: Fabio Estevam @ 2019-09-19 11:37 UTC (permalink / raw)
To: Philipp Puschmann
Cc: Fugang Duan, Sascha Hauer, linux-kernel, Vinod, NXP Linux Team,
Sascha Hauer, Dan Williams, Robin Gong, Shawn Guo, dmaengine,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Lucas Stach
In-Reply-To: <20190919104526.29851-2-philipp.puschmann@emlix.com>
Hi Philipp,
On Thu, Sep 19, 2019 at 7:45 AM Philipp Puschmann
<philipp.puschmann@emlix.com> wrote:
>
> BD_DONE flag marks ownership of the buffer. When 1 SDMA owns the
> buffer, when 0 ARM owns it. When processing the buffers in
> sdma_update_channel_loop the ownership of the currently processed
> buffer was set to SDMA again before running the callback function of
> the buffer and while the sdma script may be running in parallel. So
> there was the possibility to get the buffer overwritten by SDMA before
> it has been processed by kernel leading to kind of random errors in the
> upper layers, e.g. bluetooth.
>
> Fixes: broken since start
The Fixes tag requires a commit ID like this:
Fixes: 1ec1e82f2510 ("dmaengine: Add Freescale i.MX SDMA support")
Same applies to the other patch.
_______________________________________________
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 v3 4/6] psci: Add hvc call service for ptp_kvm.
From: Marc Zyngier @ 2019-09-19 11:39 UTC (permalink / raw)
To: Paolo Bonzini, Jianyong Wu (Arm Technology China),
netdev@vger.kernel.org, yangbo.lu@nxp.com, john.stultz@linaro.org,
tglx@linutronix.de, sean.j.christopherson@intel.com,
richardcochran@gmail.com, Mark Rutland, Will Deacon,
Suzuki Poulose
Cc: Justin He (Arm Technology China), kvm@vger.kernel.org,
Steve Capper, linux-kernel@vger.kernel.org,
Kaly Xin (Arm Technology China), nd,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <ef6ab8bd-41ad-88f8-9cfd-dc749ca65310@redhat.com>
On 19/09/2019 12:07, Paolo Bonzini wrote:
> On 19/09/19 11:46, Jianyong Wu (Arm Technology China) wrote:
>>> On 18/09/19 11:57, Jianyong Wu (Arm Technology China) wrote:
>>>> Paolo Bonzini wrote:
>>>>> This is not Y2038-safe. Please use ktime_get_real_ts64 instead, and
>>>>> split the 64-bit seconds value between val[0] and val[1].
>>
>> Val[] should be long not u32 I think, so in arm64 I can avoid that Y2038_safe, but
>> also need rewrite for arm32.
>
> I don't think there's anything inherently wrong with u32 val[], and as
> you notice it lets you reuse code between arm and arm64. It's up to you
> and Marc to decide.
>
>>>>> However, it seems to me that the new function is not needed and you
>>>>> can just use ktime_get_snapshot. You'll get the time in
>>>>> systime_snapshot->real and the cycles value in systime_snapshot->cycles.
>>>>
>>>> See patch 5/6, I need both counter cycle and clocksource,
>>> ktime_get_snapshot seems only offer cycles.
>>>
>>> No, patch 5/6 only needs the current clock (ptp_sc.cycles is never accessed).
>>> So you could just use READ_ONCE(tk->tkr_mono.clock).
>>>
>> Yeah, patch 5/6 just need clocksource, but I think tk->tkr_mono.clock can't read in external like module,
>> So I need an API to expose clocksource.
>>
>>> However, even then I don't think it is correct to use ptp_sc.cs blindly in patch
>>> 5. I think there is a misunderstanding on the meaning of
>>> system_counterval.cs as passed to get_device_system_crosststamp.
>>> system_counterval.cs is not the active clocksource; it's the clocksource on
>>> which system_counterval.cycles is based.
>>>
>>
>> I think we can use system_counterval_t as pass current clocksource to system_counterval_t.cs and its
>> corresponding cycles to system_counterval_t.cycles. is it a big problem?
>
> Yes, it is. Because...
>
>>> Hypothetically, the clocksource could be one for which ptp_sc.cycles is _not_
>>> a cycle value. If you set system_counterval.cs to the system clocksource,
>>> get_device_system_crosststamp will return a bogus value.
>>
>> Yeah, but in patch 3/6, we have a corresponding pair of clock source and cycle value. So I think there will be no
>> that problem in this patch set.
>> In the implementation of get_device_system_crosststamp:
>> "
>> ...
>> if (tk->tkr_mono.clock != system_counterval.cs)
>> return -ENODEV;
>> ...
>> "
>> We need tk->tkr_mono.clock passed to get_device_system_crosststamp, just like patch 3/6 do, otherwise will return error.
>
> ... if the hypercall returns an architectural timer value, you must not
> pass tk->tkr.mono.clock to get_device_system_crosststamp: you must pass
> &clocksource_counter. This way, PTP is disabled when using any other
> clocksource.
>
>>> So system_counterval.cs should be set to something like
>>> &clocksource_counter (from drivers/clocksource/arm_arch_timer.c).
>>> Perhaps the right place to define kvm_arch_ptp_get_clock_fn is in that file?
>>>
>> I have checked that ptp_sc.cs is arch_sys_counter.
>> Also move the module API to arm_arch_timer.c will looks a little
>> ugly and it's not easy to be accept by arm side I think.
>
> I don't think it's ugly but more important, using tk->tkr_mono.clock is
> incorrect. See how the x86 code hardcodes &kvm_clock, it's the same for
> ARM.
Not really. The guest kernel is free to use any clocksource it wishes.
In some cases, it is actually desirable (like these broken systems that
cannot use an in-kernel irqchip...). Maybe it is that on x86 the guest
only uses the kvm_clock, but that's a much harder sell on ARM. The fact
that ptp_kvm assumes that the clocksource is fixed doesn't seem correct
in that case.
M.
--
Jazz is not dead, it just smells funny...
_______________________________________________
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] serial: imx: adapt rx buffer and dma periods
From: Philipp Puschmann @ 2019-09-19 11:40 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: festevam, fugang.duan, linux-serial, gregkh, s.hauer,
linux-kernel, linux-imx, kernel, jslaby, yibin.gong, shawnguo,
linux-arm-kernel, l.stach
In-Reply-To: <20190919112216.qjkx5wvqhsadjxg5@pengutronix.de>
Hi Uwe
Am 19.09.19 um 13:22 schrieb Uwe Kleine-König:
> On Thu, Sep 19, 2019 at 12:26:28PM +0200, Philipp Puschmann wrote:
>> Using only 4 DMA periods for UART RX is very few if we have a high
>> frequency of small transfers - like in our case using Bluetooth with
>> many small packets via UART - causing many dma transfers but in each
>> only filling a fraction of a single buffer. Such a case may lead to
>> the situation that DMA RX transfer is triggered but no free buffer is
>> available. While we have addressed the dma handling already with
>> "dmaengine: imx-sdma: fix dma freezes" we still want to avoid
>
> Is this statement still true now that you split this patch out of your
> bigger series?
Yes. The dma patches care about stopping DMA channel. This patch tries to
avoid that the channel runs out of usable buffers (aka dma periods).
>
>> UART RX FIFO overrun. So we decrease the size of the buffers and
>> increase their number and the total buffer size.
>
> What happens when such an RX FIFO overrun happens? Are characters lost?
> Or only time?
Good question. In explanation i have missed an important point:
When using HW flowcontrol via RTS/CTS and the buffer is full CTS is used to
tell the remote device - here the Bluetooth chip - to stop sending data.
For a while this prevents losing of characters. But then the remote device
comes into trouble as its internal TX buffers runs over. Depends on the
device how it handles this case and if it recovers if data flow is enabled
again.
In case without HW flow control characters would be lost. Depends on the upper
layer what happens then.
> Does your change have an influence if I do fewer but
> bigger transfers?
Don't think so. The dma periods are raw data buffers. If one is full the next one
is being used. For the performance i don't see a significant difference between
using 1 kB buffers or 4 kB buffers.
Regards,
Philipp
>
> Best regards
> Uwe
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/3] dmaengine: imx-sdma: fix buffer ownership
From: Philipp Puschmann @ 2019-09-19 11:42 UTC (permalink / raw)
To: Fabio Estevam
Cc: Fugang Duan, Sascha Hauer, linux-kernel, Vinod, NXP Linux Team,
Sascha Hauer, Dan Williams, Robin Gong, Shawn Guo, dmaengine,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Lucas Stach
In-Reply-To: <CAOMZO5BNvejzMxhZiaJ36E5XES=uVNn_G-+fXQfStzy5W+YbsA@mail.gmail.com>
Hi Fabio,
Am 19.09.19 um 13:37 schrieb Fabio Estevam:
> Hi Philipp,
>
> On Thu, Sep 19, 2019 at 7:45 AM Philipp Puschmann
> <philipp.puschmann@emlix.com> wrote:
>>
>> BD_DONE flag marks ownership of the buffer. When 1 SDMA owns the
>> buffer, when 0 ARM owns it. When processing the buffers in
>> sdma_update_channel_loop the ownership of the currently processed
>> buffer was set to SDMA again before running the callback function of
>> the buffer and while the sdma script may be running in parallel. So
>> there was the possibility to get the buffer overwritten by SDMA before
>> it has been processed by kernel leading to kind of random errors in the
>> upper layers, e.g. bluetooth.
>>
>> Fixes: broken since start
>
> The Fixes tag requires a commit ID like this:
>
> Fixes: 1ec1e82f2510 ("dmaengine: Add Freescale i.MX SDMA support")
>
> Same applies to the other patch.
>
oh okay, thanks. I will fix this with v4.
Regards,
Philipp
_______________________________________________
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 v1 4/9] ASoC: wm8994: Add support for MCLKn clock gating
From: Sylwester Nawrocki @ 2019-09-19 11:58 UTC (permalink / raw)
To: Charles Keepax
Cc: devicetree, alsa-devel, linux-samsung-soc, b.zolnierkie, sbkim73,
patches, lgirdwood, krzk, robh+dt, broonie, linux-arm-kernel,
m.szyprowski
In-Reply-To: <20190918143157.GH10204@ediswmail.ad.cirrus.com>
On 9/18/19 16:31, Charles Keepax wrote:
>> @@ -2315,6 +2396,8 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
>>
>> active_dereference(component);
>> }
>> + if (mclk)
>> + clk_disable_unprepare(mclk);
>
> I don't think this works in the case of changing active FLLs.
> The driver looks like it allows changing the FLL configuration
> whilst the FLL is already active in which case it you would have
> two wm8994_set_fll calls enabling the FLL but only a single one
> disabling it. Resulting in the FLL being off but the MCLK being
> left enabled.
Indeed I missed this scenario, or rather assumed it won't be used.
But since the driver allows reconfiguring active FLLs we should make
sure such use case remains properly supported.
What I came up so far as a fix is reading current FLL refclk source and
if FLL was enabled with that source disabling refclk, before we change FLL
configuration to new one. So we have clk_disable_unprepare(MCLK) more
closely following FLL enable bit changes. I have tested it and it seems
to work - something like below. Do you think it makes sense?
--
Regards,
Sylwester
---------8<----------
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index bf02e8908d5a..78a0835a095e 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -2277,6 +2277,31 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_1 + reg_offset,
WM8994_FLL1_ENA, 0);
+ /* Disable MCLK if needed before we possibly change to new clock parent */
+ if (was_enabled) {
+ reg = snd_soc_component_read32(component, WM8994_FLL1_CONTROL_5
+ + reg_offset);
+ reg = ((reg & WM8994_FLL1_REFCLK_SRC_MASK)
+ >> WM8994_FLL1_REFCLK_SRC_SHIFT) + 1;
+
+ switch (reg) {
+ case WM8994_FLL_SRC_MCLK1:
+ mclk = control->mclk[WM8994_MCLK1].clk;
+ break;
+ case WM8994_FLL_SRC_MCLK2:
+ mclk = control->mclk[WM8994_MCLK2].clk;
+ break;
+ default:
+ mclk = NULL;
+ }
+
+ clk_disable_unprepare(mclk);
+ }
+
if (wm8994->fll_byp && src == WM8994_FLL_SRC_BCLK &&
freq_in == freq_out && freq_out) {
dev_dbg(component->dev, "Bypassing FLL%d\n", id + 1);
@@ -2396,8 +2420,6 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
active_dereference(component);
}
- if (mclk)
- clk_disable_unprepare(mclk);
}
out:
---------8<----------
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v1 7/9] ASoC: samsung: arndale: Add support for WM1811 CODEC
From: Sylwester Nawrocki @ 2019-09-19 12:00 UTC (permalink / raw)
To: Charles Keepax
Cc: devicetree, alsa-devel, linux-samsung-soc, b.zolnierkie, sbkim73,
patches, lgirdwood, krzk, robh+dt, broonie, linux-arm-kernel,
m.szyprowski
In-Reply-To: <20190918144553.GJ10204@ediswmail.ad.cirrus.com>
On 9/18/19 16:45, Charles Keepax wrote:
> If your removing the of_match_ptr below I think the
> __maybe_unused should be removed as well.
Good point, I will remove the now unneeded __maybe_unused as well.
--
Thanks,
Sylwester
_______________________________________________
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