* Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table
From: Niklas Cassel @ 2019-07-25 10:40 UTC (permalink / raw)
To: Viresh Kumar
Cc: Andy Gross, linux-arm-msm, jorge.ramirez-ortiz, sboyd, vireshk,
bjorn.andersson, ulf.hansson, Rob Herring, Mark Rutland,
devicetree, linux-kernel
In-Reply-To: <20190723015635.rl5a2isjnjn23fzh@vireshk-i7>
On Tue, Jul 23, 2019 at 07:26:35AM +0530, Viresh Kumar wrote:
> On 19-07-19, 17:45, Niklas Cassel wrote:
> > Hello Viresh,
> >
> > Could you please have a look at the last two patches here:
> > https://git.linaro.org/people/niklas.cassel/kernel.git/log/?h=cpr-opp-hz
>
> There is no sane way of providing review comments with a link to the
> git tree :)
>
> I still had a look and I see that you don't search for max frequency
> but just any OPP that has required-opps set to the level u want. Also,
> can't there be multiple phandles in required-opps in your case ?
For each OPP in the CPR OPP table, we need three things,
opp-level, qcom,fuse-level and opp-hz.
The first two can simply be parsed from the OPP node
itself while iterating the CPR OPP table.
The opp-hz has to be fetched from the CPU OPP table.
Several OPPs might have the same qcom,fuse-level value.
However, they will have unique opp-level values and unique
opp-hz values. Each opp-level has a matching opp-hz.
required-opps is basically a connection between a opp-hz
(CPU OPP table) and and a opp-level (CPR OPP table).
So there will be only one match. No need to search for
max frequency.
I think you are confusing this with something else.
The CPR hardware has to be programmed with the highest
frequency for each qcom,fuse-corner.
This is done here:
https://git.linaro.org/people/niklas.cassel/kernel.git/tree/drivers/power/avs/qcom-cpr.c?h=cpr-full#n1219
by saving the highest frequency for each fuse level
while iterating the OPP table.
There can be only one phandle in the required-opps in my case,
this is one of the reasons why I prefer implementing it in the
CPR driver. If it were to be implemented in OPP core, it probably
has to handle multiple phandles.
>
> > If you like my proposal then I could send out the first patch (the one to
> > OPP core) as a real patch (with an improved commit message), and
> > incorporate the second patch into my CPR patch series when I send out a V2.
>
> Send them both in your series only, otherwise the first one is useless
> anyway.
Ok, will do.
Kind regards,
Niklas
^ permalink raw reply
* Re: [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
From: Peter De Schrijver @ 2019-07-25 10:38 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland, pgaikwad,
sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <20190725103348.GN12715@pdeschrijver-desktop.Nvidia.com>
On Thu, Jul 25, 2019 at 01:33:48PM +0300, Peter De Schrijver wrote:
> On Thu, Jul 25, 2019 at 01:05:13PM +0300, Dmitry Osipenko wrote:
> > 25.07.2019 12:55, Peter De Schrijver пишет:
> > > On Mon, Jul 22, 2019 at 12:54:51PM +0300, Dmitry Osipenko wrote:
> > >>
> > >> All Tegra SoCs support SC7, hence the 'supports_sc7' and the comment
> > >> doesn't sound correct to me. Something like 'firmware_sc7' should suit
> > >> better here.
> > >>
> > >>> + writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
> > >>
> > >> Secondly, I'm also not sure why COP interrupts need to be disabled for
> > >> pre-T210 at all, since COP is unused. This looks to me like it was
> > >> cut-n-pasted from downstream kernel without a good reason and could be
> > >> simply removed.
> > >
> > > I don't think we can rely on the fact that COP is unused. People can
> > > write their own code to run on COP.
> >
> > 1. Not upstream - doesn't matter.
> >
>
> The code is not part of the kernel, so obviously it's not upstream?
>
> > 2. That's not very good if something unknown is running on COP and then
> > kernel suddenly intervenes, don't you think so?
>
> Unless the code was written with this in mind.
>
Looking at this again, I don't think we need to enable the IRQ at all.
Peter.
^ permalink raw reply
* Re: [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
From: Peter De Schrijver @ 2019-07-25 10:33 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland, pgaikwad,
sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <dd01be5d-bab9-1329-c7ac-c3c893d49dd1@gmail.com>
On Thu, Jul 25, 2019 at 01:05:13PM +0300, Dmitry Osipenko wrote:
> 25.07.2019 12:55, Peter De Schrijver пишет:
> > On Mon, Jul 22, 2019 at 12:54:51PM +0300, Dmitry Osipenko wrote:
> >>
> >> All Tegra SoCs support SC7, hence the 'supports_sc7' and the comment
> >> doesn't sound correct to me. Something like 'firmware_sc7' should suit
> >> better here.
> >>
> >>> + writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
> >>
> >> Secondly, I'm also not sure why COP interrupts need to be disabled for
> >> pre-T210 at all, since COP is unused. This looks to me like it was
> >> cut-n-pasted from downstream kernel without a good reason and could be
> >> simply removed.
> >
> > I don't think we can rely on the fact that COP is unused. People can
> > write their own code to run on COP.
>
> 1. Not upstream - doesn't matter.
>
The code is not part of the kernel, so obviously it's not upstream?
> 2. That's not very good if something unknown is running on COP and then
> kernel suddenly intervenes, don't you think so?
Unless the code was written with this in mind.
Peter.
^ permalink raw reply
* [PATCH 3/3] media: Add support for Cadence CSI2RX 2.1
From: Jan Kotas @ 2019-07-25 10:26 UTC (permalink / raw)
To: maxime.ripard, mchehab, robh+dt, mark.rutland
Cc: rafalc, linux-media, devicetree, linux-kernel, Jan Kotas
In-Reply-To: <20190725102648.13445-1-jank@cadence.com>
This patch adds support for CSI2RX v2.1 version of the controller.
Signed-off-by: Jan Kotas <jank@cadence.com>
---
drivers/media/platform/cadence/cdns-csi2rx.c | 137 ++++++++++++++++++++++-----
1 file changed, 115 insertions(+), 22 deletions(-)
diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 97ec09e72..4c76c6490 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -26,6 +26,9 @@
#define CSI2RX_SOFT_RESET_PROTOCOL BIT(1)
#define CSI2RX_SOFT_RESET_FRONT BIT(0)
+#define CSI2RX_V2_CORE_CTRL_REG 0x004
+#define CSI2RX_V2_CORE_CTRL_START BIT(0)
+
#define CSI2RX_STATIC_CFG_REG 0x008
#define CSI2RX_STATIC_CFG_DLANE_MAP(llane, plane) ((plane) << (16 + (llane) * 4))
#define CSI2RX_STATIC_CFG_LANES_MASK GENMASK(11, 8)
@@ -54,6 +57,15 @@ enum csi2rx_pads {
CSI2RX_PAD_MAX,
};
+struct csi2rx_priv;
+
+/* CSI2RX Variant Operations */
+struct csi2rx_vops {
+ void (*get_dev_cfg)(struct csi2rx_priv *csi2rx);
+ void (*reset)(struct csi2rx_priv *csi2rx);
+ void (*map_static)(struct csi2rx_priv *csi2rx);
+};
+
struct csi2rx_priv {
struct device *dev;
unsigned int count;
@@ -69,6 +81,7 @@ struct csi2rx_priv {
struct clk *p_clk;
struct clk *pixel_clk[CSI2RX_STREAMS_MAX];
struct phy *dphy;
+ struct csi2rx_vops *vops;
u8 lanes[CSI2RX_LANES_MAX];
u8 num_lanes;
@@ -92,6 +105,32 @@ struct csi2rx_priv *v4l2_subdev_to_csi2rx(struct v4l2_subdev *subdev)
return container_of(subdev, struct csi2rx_priv, subdev);
}
+static void csi2rx_get_dev_cfg(struct csi2rx_priv *csi2rx)
+{
+ u32 dev_cfg;
+
+ clk_prepare_enable(csi2rx->p_clk);
+ dev_cfg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG);
+ clk_disable_unprepare(csi2rx->p_clk);
+
+ csi2rx->max_lanes = dev_cfg & 7;
+ csi2rx->max_streams = (dev_cfg >> 4) & 7;
+ csi2rx->has_internal_dphy = dev_cfg & BIT(3) ? true : false;
+}
+
+static void csi2rx_v2_get_dev_cfg(struct csi2rx_priv *csi2rx)
+{
+ u32 dev_cfg;
+
+ clk_prepare_enable(csi2rx->p_clk);
+ dev_cfg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG);
+ clk_disable_unprepare(csi2rx->p_clk);
+
+ csi2rx->max_lanes = dev_cfg & 0xF;
+ csi2rx->max_streams = (dev_cfg >> 5) & 0xF;
+ csi2rx->has_internal_dphy = dev_cfg & BIT(4) ? true : false;
+}
+
static void csi2rx_reset(struct csi2rx_priv *csi2rx)
{
writel(CSI2RX_SOFT_RESET_PROTOCOL | CSI2RX_SOFT_RESET_FRONT,
@@ -102,18 +141,21 @@ static void csi2rx_reset(struct csi2rx_priv *csi2rx)
writel(0, csi2rx->base + CSI2RX_SOFT_RESET_REG);
}
-static int csi2rx_start(struct csi2rx_priv *csi2rx)
+static void csi2rx_v2_reset(struct csi2rx_priv *csi2rx)
+{
+ writel(0, csi2rx->base + CSI2RX_V2_CORE_CTRL_REG);
+
+ udelay(10);
+
+ writel(CSI2RX_V2_CORE_CTRL_START,
+ csi2rx->base + CSI2RX_V2_CORE_CTRL_REG);
+}
+
+static void csi2rx_map_static(struct csi2rx_priv *csi2rx)
{
unsigned int i;
unsigned long lanes_used = 0;
u32 reg;
- int ret;
-
- ret = clk_prepare_enable(csi2rx->p_clk);
- if (ret)
- return ret;
-
- csi2rx_reset(csi2rx);
reg = csi2rx->num_lanes << 8;
for (i = 0; i < csi2rx->num_lanes; i++) {
@@ -135,6 +177,32 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
}
writel(reg, csi2rx->base + CSI2RX_STATIC_CFG_REG);
+}
+
+static void csi2rx_v2_map_static(struct csi2rx_priv *csi2rx)
+{
+ u32 reg;
+
+ reg = csi2rx->num_lanes << 4;
+ writel(reg, csi2rx->base + CSI2RX_STATIC_CFG_REG);
+}
+
+static int csi2rx_start(struct csi2rx_priv *csi2rx)
+{
+ unsigned int i;
+ int ret;
+
+ ret = clk_prepare_enable(csi2rx->p_clk);
+ if (ret)
+ return ret;
+
+ if (csi2rx->vops) {
+ if (csi2rx->vops->reset)
+ csi2rx->vops->reset(csi2rx);
+
+ if (csi2rx->vops->map_static)
+ csi2rx->vops->map_static(csi2rx);
+ }
ret = v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, true);
if (ret)
@@ -282,7 +350,6 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx,
{
struct resource *res;
unsigned char i;
- u32 dev_cfg;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
csi2rx->base = devm_ioremap_resource(&pdev->dev, res);
@@ -316,26 +383,25 @@ static int csi2rx_get_resources(struct csi2rx_priv *csi2rx,
return -EINVAL;
}
- clk_prepare_enable(csi2rx->p_clk);
- dev_cfg = readl(csi2rx->base + CSI2RX_DEVICE_CFG_REG);
- clk_disable_unprepare(csi2rx->p_clk);
+ if (csi2rx->vops && csi2rx->vops->get_dev_cfg) {
+ csi2rx->vops->get_dev_cfg(csi2rx);
+ } else {
+ dev_err(&pdev->dev, "Couldn't get device configuration\n");
+ return -EINVAL;
+ }
- csi2rx->max_lanes = dev_cfg & 7;
if (csi2rx->max_lanes > CSI2RX_LANES_MAX) {
dev_err(&pdev->dev, "Invalid number of lanes: %u\n",
csi2rx->max_lanes);
return -EINVAL;
}
- csi2rx->max_streams = (dev_cfg >> 4) & 7;
if (csi2rx->max_streams > CSI2RX_STREAMS_MAX) {
dev_err(&pdev->dev, "Invalid number of streams: %u\n",
csi2rx->max_streams);
return -EINVAL;
}
- csi2rx->has_internal_dphy = dev_cfg & BIT(3) ? true : false;
-
/*
* FIXME: Once we'll have internal D-PHY support, the check
* will need to be removed.
@@ -426,9 +492,39 @@ static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)
return ret;
}
+static const struct csi2rx_vops csi2rx_vops = {
+ .get_dev_cfg = csi2rx_get_dev_cfg,
+ .reset = csi2rx_reset,
+ .map_static = csi2rx_map_static
+};
+
+static const struct csi2rx_vops csi2rx_v2_vops = {
+ .get_dev_cfg = csi2rx_v2_get_dev_cfg,
+ .reset = csi2rx_v2_reset,
+ .map_static = csi2rx_v2_map_static
+};
+
+static const struct of_device_id csi2rx_of_table[] = {
+ {
+ .compatible = "cdns,csi2rx",
+ .data = &csi2rx_vops
+ },
+ {
+ .compatible = "cdns,csi2rx-1.3",
+ .data = &csi2rx_vops
+ },
+ {
+ .compatible = "cdns,csi2rx-2.1",
+ .data = &csi2rx_v2_vops
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, csi2rx_of_table);
+
static int csi2rx_probe(struct platform_device *pdev)
{
struct csi2rx_priv *csi2rx;
+ const struct of_device_id *of_id;
unsigned int i;
int ret;
@@ -439,6 +535,9 @@ static int csi2rx_probe(struct platform_device *pdev)
csi2rx->dev = &pdev->dev;
mutex_init(&csi2rx->lock);
+ of_id = of_match_node(csi2rx_of_table, pdev->dev.of_node);
+ csi2rx->vops = (struct csi2rx_vops *)of_id->data;
+
ret = csi2rx_get_resources(csi2rx, pdev);
if (ret)
goto err_free_priv;
@@ -493,12 +592,6 @@ static int csi2rx_remove(struct platform_device *pdev)
return 0;
}
-static const struct of_device_id csi2rx_of_table[] = {
- { .compatible = "cdns,csi2rx" },
- { },
-};
-MODULE_DEVICE_TABLE(of, csi2rx_of_table);
-
static struct platform_driver csi2rx_driver = {
.probe = csi2rx_probe,
.remove = csi2rx_remove,
--
2.15.0
^ permalink raw reply related
* [PATCH 2/3] media: Add lane checks for Cadence CSI2RX
From: Jan Kotas @ 2019-07-25 10:26 UTC (permalink / raw)
To: maxime.ripard, mchehab, robh+dt, mark.rutland
Cc: rafalc, linux-media, devicetree, linux-kernel, Jan Kotas
In-Reply-To: <20190725102648.13445-1-jank@cadence.com>
This patch adds lane checks for CSI2RX, to prevent clock lane
being used as a data lane.
Signed-off-by: Jan Kotas <jank@cadence.com>
---
drivers/media/platform/cadence/cdns-csi2rx.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 31ace114e..97ec09e72 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -2,7 +2,7 @@
/*
* Driver for Cadence MIPI-CSI2 RX Controller v1.3
*
- * Copyright (C) 2017 Cadence Design Systems Inc.
+ * Copyright (C) 2017-2019 Cadence Design Systems Inc.
*/
#include <linux/clk.h>
@@ -364,7 +364,7 @@ static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)
struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 };
struct fwnode_handle *fwh;
struct device_node *ep;
- int ret;
+ int ret, i;
ep = of_graph_get_endpoint_by_regs(csi2rx->dev->of_node, 0, 0);
if (!ep)
@@ -395,6 +395,15 @@ static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)
return -EINVAL;
}
+ for (i = 0; i < csi2rx->num_lanes; i++) {
+ if (csi2rx->lanes[i] < 1) {
+ dev_err(csi2rx->dev, "Invalid lane[%d] number: %u\n",
+ i, csi2rx->lanes[i]);
+ of_node_put(ep);
+ return -EINVAL;
+ }
+ }
+
csi2rx->asd.match.fwnode = fwnode_graph_get_remote_port_parent(fwh);
csi2rx->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
of_node_put(ep);
--
2.15.0
^ permalink raw reply related
* [PATCH 1/3] media: dt-bindings: Update bindings for Cadence CSI2RX version 2.1
From: Jan Kotas @ 2019-07-25 10:26 UTC (permalink / raw)
To: maxime.ripard, mchehab, robh+dt, mark.rutland
Cc: rafalc, linux-media, devicetree, linux-kernel, Jan Kotas
In-Reply-To: <20190725102648.13445-1-jank@cadence.com>
This patch adds a DT bindings documentation for
Cadence CSI2RX v2.1 controller.
Signed-off-by: Jan Kotas <jank@cadence.com>
---
Documentation/devicetree/bindings/media/cdns,csi2rx.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/cdns,csi2rx.txt b/Documentation/devicetree/bindings/media/cdns,csi2rx.txt
index 6b02a0657..a35bb0aed 100644
--- a/Documentation/devicetree/bindings/media/cdns,csi2rx.txt
+++ b/Documentation/devicetree/bindings/media/cdns,csi2rx.txt
@@ -5,7 +5,9 @@ The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to 4 CSI
lanes in input, and 4 different pixel streams in output.
Required properties:
- - compatible: must be set to "cdns,csi2rx" and an SoC-specific compatible
+ - compatible: must be set to "cdns,csi2rx" or "cdns,csi2rx-1.3"
+ for version 1.3 of the controller, "cdns,csi2rx-2.1" for v2.1
+ and an SoC-specific compatible
- reg: base address and size of the memory mapped region
- clocks: phandles to the clocks driving the controller
- clock-names: must contain:
--
2.15.0
^ permalink raw reply related
* [PATCH 0/3] media: Add support for Cadence CSI2RX version 2.1
From: Jan Kotas @ 2019-07-25 10:26 UTC (permalink / raw)
To: maxime.ripard, mchehab, robh+dt, mark.rutland
Cc: rafalc, linux-media, devicetree, linux-kernel, Jan Kotas
This patchset adds support for Cadence CSI2RX controller version 2.1.
It currently limits maximum amount of data lanes to 4.
Existing compatibility with v1.3 is maintained.
Jan Kotas (3):
media: dt-bindings: Update bindings for Cadence CSI2RX version 2.1
media: Add lane checks for Cadence CSI2RX
media: Add support for Cadence CSI2RX 2.1
.../devicetree/bindings/media/cdns,csi2rx.txt | 4 +-
drivers/media/platform/cadence/cdns-csi2rx.c | 150 +++++++++++++++++----
2 files changed, 129 insertions(+), 25 deletions(-)
--
2.15.0
^ permalink raw reply
* Re: [PATCH 0/4] ARM: dts: stm32: enable FMC2 NAND controller on stm32mp157c-ev1
From: Alexandre Torgue @ 2019-07-25 10:20 UTC (permalink / raw)
To: Christophe Kerello, robh+dt, mark.rutland, linux, olof, arnd
Cc: mcoquelin.stm32, linux-stm32, linux-kernel, devicetree,
linux-arm-kernel
In-Reply-To: <1561128590-14621-1-git-send-email-christophe.kerello@st.com>
Hi Christophe
On 6/21/19 4:49 PM, Christophe Kerello wrote:
> This patchset adds and enables FMC2 NAND controller used on
> stm32mp157c-ev1.
>
> Christophe Kerello (4):
> ARM: dts: stm32: add FMC2 NAND controller support on stm32mp157c
> ARM: dts: stm32: add FMC2 NAND controller pins muxing on
> stm32mp157c-ev1
> ARM: dts: stm32: enable FMC2 NAND controller on stm32mp157c-ev1
> ARM: multi_v7_defconfig: add FMC2 NAND controller support
>
> arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 44 +++++++++++++++++++++++++++++++
> arch/arm/boot/dts/stm32mp157c-ev1.dts | 16 +++++++++++
> arch/arm/boot/dts/stm32mp157c.dtsi | 19 +++++++++++++
> arch/arm/configs/multi_v7_defconfig | 1 +
> 4 files changed, 80 insertions(+)
>
Series applied on stm32-next.
Note, I changed capital letter in patch1 directly (As I responded late).
Regards
Alex
^ permalink raw reply
* Re: [PATCH v3 3/5] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
From: Chanwoo Choi @ 2019-07-25 10:17 UTC (permalink / raw)
To: k.konieczny
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Krzysztof Kozlowski,
Kukjin Kim, Kyungmin Park, Mark Rutland, MyungJoo Ham,
Nishanth Menon, Rob Herring, Stephen Boyd, Viresh Kumar,
devicetree, linux-arm-kernel, linux-kernel, linux-pm,
linux-samsung-soc
In-Reply-To: <20190719150535.15501-4-k.konieczny@partner.samsung.com>
Hi Kamil,
Looks good to me. But, I have some comment. Please check them.
After this patch, exynos_bus_target is perfectly same with
exynos_bus_passive_target. The exynos_bus_passive_target() could be removed.
On 19. 7. 20. 오전 12:05, k.konieczny@partner.samsung.com wrote:
> Reuse opp core code for setting bus clock and voltage. As a side
> effect this allow useage of coupled regulators feature (required
s/useage/usage ?
> for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
> uses regulator_set_voltage_triplet() for setting regulator voltage
> while the old code used regulator_set_voltage_tol() with fixed
> tolerance. This patch also removes no longer needed parsing of DT
> property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
> it).
>
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
> drivers/devfreq/exynos-bus.c | 143 +++++++++--------------------------
> 1 file changed, 37 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index f391044aa39d..c2147b0912a0 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -25,7 +25,6 @@
> #include <linux/slab.h>
>
> #define DEFAULT_SATURATION_RATIO 40
> -#define DEFAULT_VOLTAGE_TOLERANCE 2
>
> struct exynos_bus {
> struct device *dev;
> @@ -37,9 +36,9 @@ struct exynos_bus {
>
> unsigned long curr_freq;
>
> - struct regulator *regulator;
> + struct opp_table *opp_table;
> +
> struct clk *clk;
> - unsigned int voltage_tolerance;
> unsigned int ratio;
> };
>
> @@ -99,56 +98,23 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
> {
> struct exynos_bus *bus = dev_get_drvdata(dev);
> struct dev_pm_opp *new_opp;
> - unsigned long old_freq, new_freq, new_volt, tol;
> int ret = 0;
>
> - /* Get new opp-bus instance according to new bus clock */
> + /* Get correct frequency for bus. */
> new_opp = devfreq_recommended_opp(dev, freq, flags);
> if (IS_ERR(new_opp)) {
> dev_err(dev, "failed to get recommended opp instance\n");
> return PTR_ERR(new_opp);
> }
>
> - new_freq = dev_pm_opp_get_freq(new_opp);
> - new_volt = dev_pm_opp_get_voltage(new_opp);
> dev_pm_opp_put(new_opp);
>
> - old_freq = bus->curr_freq;
> -
> - if (old_freq == new_freq)
> - return 0;
> - tol = new_volt * bus->voltage_tolerance / 100;
> -
> /* Change voltage and frequency according to new OPP level */
> mutex_lock(&bus->lock);
> + ret = dev_pm_opp_set_rate(dev, *freq);
> + if (!ret)
> + bus->curr_freq = *freq;
>
> - if (old_freq < new_freq) {
> - ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
> - if (ret < 0) {
> - dev_err(bus->dev, "failed to set voltage\n");
> - goto out;
> - }
> - }
> -
> - ret = clk_set_rate(bus->clk, new_freq);
> - if (ret < 0) {
> - dev_err(dev, "failed to change clock of bus\n");
> - clk_set_rate(bus->clk, old_freq);
> - goto out;
> - }
> -
> - if (old_freq > new_freq) {
> - ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
> - if (ret < 0) {
> - dev_err(bus->dev, "failed to set voltage\n");
> - goto out;
> - }
> - }
> - bus->curr_freq = new_freq;
> -
> - dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
> - old_freq, new_freq, clk_get_rate(bus->clk));
> -out:
> mutex_unlock(&bus->lock);
>
> return ret;
> @@ -195,8 +161,8 @@ static void exynos_bus_exit(struct device *dev)
> dev_warn(dev, "failed to disable the devfreq-event devices\n");
>
> clk_disable_unprepare(bus->clk);
> - if (bus->regulator)
> - regulator_disable(bus->regulator);
> + if (bus->opp_table)
> + dev_pm_opp_put_regulators(bus->opp_table);
>
> dev_pm_opp_of_remove_table(dev);
> }
> @@ -209,39 +175,23 @@ static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
> {
> struct exynos_bus *bus = dev_get_drvdata(dev);
> struct dev_pm_opp *new_opp;
> - unsigned long old_freq, new_freq;
> - int ret = 0;
> + int ret;
>
> - /* Get new opp-bus instance according to new bus clock */
> + /* Get correct frequency for bus. */
> new_opp = devfreq_recommended_opp(dev, freq, flags);
> if (IS_ERR(new_opp)) {
> dev_err(dev, "failed to get recommended opp instance\n");
> return PTR_ERR(new_opp);
> }
>
> - new_freq = dev_pm_opp_get_freq(new_opp);
> dev_pm_opp_put(new_opp);
>
> - old_freq = bus->curr_freq;
> -
> - if (old_freq == new_freq)
> - return 0;
> -
> /* Change the frequency according to new OPP level */
> mutex_lock(&bus->lock);
> + ret = dev_pm_opp_set_rate(dev, *freq);
> + if (!ret)
> + bus->curr_freq = *freq;
>
> - ret = clk_set_rate(bus->clk, new_freq);
> - if (ret < 0) {
> - dev_err(dev, "failed to set the clock of bus\n");
> - goto out;
> - }
> -
> - *freq = new_freq;
> - bus->curr_freq = new_freq;
> -
> - dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
> - old_freq, new_freq, clk_get_rate(bus->clk));
> -out:
> mutex_unlock(&bus->lock);
>
> return ret;
> @@ -259,20 +209,9 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
> struct exynos_bus *bus)
> {
> struct device *dev = bus->dev;
> - int i, ret, count, size;
> -
> - /* Get the regulator to provide each bus with the power */
> - bus->regulator = devm_regulator_get(dev, "vdd");
> - if (IS_ERR(bus->regulator)) {
> - dev_err(dev, "failed to get VDD regulator\n");
> - return PTR_ERR(bus->regulator);
> - }
> -
> - ret = regulator_enable(bus->regulator);
> - if (ret < 0) {
> - dev_err(dev, "failed to enable VDD regulator\n");
> - return ret;
> - }
> + struct opp_table *opp_table;
> + const char *vdd = "vdd";
> + int i, count, size;
>
> /*
> * Get the devfreq-event devices to get the current utilization of
> @@ -281,26 +220,29 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
> count = devfreq_event_get_edev_count(dev);
> if (count < 0) {
> dev_err(dev, "failed to get the count of devfreq-event dev\n");
> - ret = count;
> - goto err_regulator;
> + return count;
> }
> - bus->edev_count = count;
>
> + bus->edev_count = count;
> size = sizeof(*bus->edev) * count;
> bus->edev = devm_kzalloc(dev, size, GFP_KERNEL);
> - if (!bus->edev) {
> - ret = -ENOMEM;
> - goto err_regulator;
> - }
> + if (!bus->edev)
> + return -ENOMEM;
>
> for (i = 0; i < count; i++) {
> bus->edev[i] = devfreq_event_get_edev_by_phandle(dev, i);
> - if (IS_ERR(bus->edev[i])) {
> - ret = -EPROBE_DEFER;
> - goto err_regulator;
> - }
> + if (IS_ERR(bus->edev[i]))
> + return -EPROBE_DEFER;
> + }
> +
> + opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
> + if (IS_ERR(opp_table)) {
> + i = PTR_ERR(opp_table);
> + dev_err(dev, "failed to set regulators %d\n", i);
> + return i;
Maybe, you just used the 'i' defined variable instead of adding
new variable. But, I think that you better to add new variable
like 'err' for the readability. Or, jut use the 'PTR_ERR(opp_table)'
directly without any additional variable.
> }
>
> + bus->opp_table = opp_table;
Add blank line.
> /*
> * Optionally, Get the saturation ratio according to Exynos SoC
> * When measuring the utilization of each AXI bus with devfreq-event
> @@ -314,16 +256,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
> if (of_property_read_u32(np, "exynos,saturation-ratio", &bus->ratio))
> bus->ratio = DEFAULT_SATURATION_RATIO;
>
> - if (of_property_read_u32(np, "exynos,voltage-tolerance",
> - &bus->voltage_tolerance))
> - bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;
> -
> return 0;
> -
> -err_regulator:
> - regulator_disable(bus->regulator);
> -
> - return ret;
> }
>
> static int exynos_bus_parse_of(struct exynos_bus *bus)
> @@ -414,12 +347,8 @@ static int exynos_bus_probe(struct platform_device *pdev)
>
> /* Parse the device-tree to get the resource information */
> ret = exynos_bus_parse_of(bus);
> - if (ret < 0) {
> - if (!passive)
> - regulator_disable(bus->regulator);
> -
> - return ret;
> - }
> + if (ret < 0)
> + goto err_reg;
>
> if (passive)
> goto passive;
> @@ -512,10 +441,12 @@ static int exynos_bus_probe(struct platform_device *pdev)
>
> err:
> clk_disable_unprepare(bus->clk);
> - if (!passive)
> - regulator_disable(bus->regulator);
> -
> dev_pm_opp_of_remove_table(dev);
This function removes the 'opp_table'. But, the below code
uses the 'opp_table' variable by dev_pm_opp_put_regulators().
To disable the regulator, you have to call dev_pm_opp_of_remove_table(dev)
after dev_pm_opp_put_regulators(bus->opp_table).
> +err_reg:
> + if (bus->opp_table) {
> + dev_pm_opp_put_regulators(bus->opp_table);
> + bus->opp_table = NULL;
> + }
>
> return ret;
> }
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* Re: [PATCH v2 3/3] drm/bridge: sii902x: make audio mclk optional
From: Benjamin Gaignard @ 2019-07-25 10:06 UTC (permalink / raw)
To: Olivier Moysan
Cc: Mark Rutland, devicetree, jernej.skrabec, Benjamin GAIGNARD,
Jonas Karlman, David Airlie, Neil Armstrong, Jyri Sarha,
Linux Kernel Mailing List, ML dri-devel, Rob Herring,
Laurent Pinchart, linux-stm32, Linux ARM, Alexandre Torgue
In-Reply-To: <1563811560-29589-4-git-send-email-olivier.moysan@st.com>
Le lun. 22 juil. 2019 à 18:06, Olivier Moysan <olivier.moysan@st.com> a écrit :
>
> The master clock on i2s bus is not mandatory,
> as sii902X internal PLL can be used instead.
> Make use of mclk optional.
Applied on drm-misc-next.
Thanks,
Benjamin
>
> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
> Reviewed-by: Jyri Sarha <jsarha@ti.com>
> Acked-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/gpu/drm/bridge/sii902x.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 962931c20efe..a323815aa9b6 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -568,13 +568,14 @@ static int sii902x_audio_hw_params(struct device *dev, void *data,
> return ret;
> }
>
> - mclk_rate = clk_get_rate(sii902x->audio.mclk);
> -
> - ret = sii902x_select_mclk_div(&i2s_config_reg, params->sample_rate,
> - mclk_rate);
> - if (mclk_rate != ret * params->sample_rate)
> - dev_dbg(dev, "Inaccurate reference clock (%ld/%d != %u)\n",
> - mclk_rate, ret, params->sample_rate);
> + if (sii902x->audio.mclk) {
> + mclk_rate = clk_get_rate(sii902x->audio.mclk);
> + ret = sii902x_select_mclk_div(&i2s_config_reg,
> + params->sample_rate, mclk_rate);
> + if (mclk_rate != ret * params->sample_rate)
> + dev_dbg(dev, "Inaccurate reference clock (%ld/%d != %u)\n",
> + mclk_rate, ret, params->sample_rate);
> + }
>
> mutex_lock(&sii902x->mutex);
>
> @@ -751,11 +752,11 @@ static int sii902x_audio_codec_init(struct sii902x *sii902x,
> sii902x->audio.i2s_fifo_sequence[i] |= audio_fifo_id[i] |
> i2s_lane_id[lanes[i]] | SII902X_TPI_I2S_FIFO_ENABLE;
>
> - sii902x->audio.mclk = devm_clk_get(dev, "mclk");
> + sii902x->audio.mclk = devm_clk_get_optional(dev, "mclk");
> if (IS_ERR(sii902x->audio.mclk)) {
> dev_err(dev, "%s: No clock (audio mclk) found: %ld\n",
> __func__, PTR_ERR(sii902x->audio.mclk));
> - return 0;
> + return PTR_ERR(sii902x->audio.mclk);
> }
>
> sii902x->audio.pdev = platform_device_register_data(
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: display: sii902x: Change audio mclk binding
From: Benjamin Gaignard @ 2019-07-25 10:05 UTC (permalink / raw)
To: Olivier Moysan
Cc: Mark Rutland, devicetree, jernej.skrabec, Benjamin GAIGNARD,
Jonas Karlman, David Airlie, Neil Armstrong, Jyri Sarha,
Linux Kernel Mailing List, ML dri-devel, Rob Herring,
Laurent Pinchart, linux-stm32, Linux ARM, Alexandre Torgue
In-Reply-To: <1563811560-29589-3-git-send-email-olivier.moysan@st.com>
Le lun. 22 juil. 2019 à 18:06, Olivier Moysan <olivier.moysan@st.com> a écrit :
>
> As stated in SiL9022/24 datasheet, master clock is not required for I2S.
> Make mclk property optional in DT bindings.
>
> Fixes: 3f18021f43a3 ("dt-bindings: display: sii902x: Add HDMI audio bindings")
>
Applied on drm-misc-next.
Thanks,
Benjamin
> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
> Reviewed-by: Jyri Sarha <jsarha@ti.com>
> Acked-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Documentation/devicetree/bindings/display/bridge/sii902x.txt | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> index 2df44b7d3821..6e14e087c0d0 100644
> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> @@ -26,9 +26,8 @@ Optional properties:
> - clocks: phandle and clock specifier for each clock listed in
> the clock-names property
> - clock-names: "mclk"
> - Describes SII902x MCLK input. MCLK is used to produce
> - HDMI audio CTS values. This property is required if
> - "#sound-dai-cells"-property is present. This property follows
> + Describes SII902x MCLK input. MCLK can be used to produce
> + HDMI audio CTS values. This property follows
> Documentation/devicetree/bindings/clock/clock-bindings.txt
> consumer binding.
>
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 1/3] drm/bridge: sii902x: fix missing reference to mclk clock
From: Benjamin Gaignard @ 2019-07-25 10:05 UTC (permalink / raw)
To: Olivier Moysan
Cc: Mark Rutland, devicetree, jernej.skrabec, Benjamin GAIGNARD,
Jonas Karlman, David Airlie, Neil Armstrong, Jyri Sarha,
Linux Kernel Mailing List, ML dri-devel, Rob Herring,
Laurent Pinchart, linux-stm32, Linux ARM, Alexandre Torgue
In-Reply-To: <1563811560-29589-2-git-send-email-olivier.moysan@st.com>
Le lun. 22 juil. 2019 à 18:06, Olivier Moysan <olivier.moysan@st.com> a écrit :
>
> Add devm_clk_get call to retrieve reference to master clock.
>
> Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
>
> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
> Reviewed-by: Jyri Sarha <jsarha@ti.com>
> Acked-by: Andrzej Hajda <a.hajda@samsung.com
Applied on drm-misc-next.
Thanks,
Benjamin
> ---
> drivers/gpu/drm/bridge/sii902x.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index c2f97e5997a1..962931c20efe 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -751,6 +751,7 @@ static int sii902x_audio_codec_init(struct sii902x *sii902x,
> sii902x->audio.i2s_fifo_sequence[i] |= audio_fifo_id[i] |
> i2s_lane_id[lanes[i]] | SII902X_TPI_I2S_FIFO_ENABLE;
>
> + sii902x->audio.mclk = devm_clk_get(dev, "mclk");
> if (IS_ERR(sii902x->audio.mclk)) {
> dev_err(dev, "%s: No clock (audio mclk) found: %ld\n",
> __func__, PTR_ERR(sii902x->audio.mclk));
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
From: Dmitry Osipenko @ 2019-07-25 10:05 UTC (permalink / raw)
To: Peter De Schrijver
Cc: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland, pgaikwad,
sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <20190725095502.GM12715@pdeschrijver-desktop.Nvidia.com>
25.07.2019 12:55, Peter De Schrijver пишет:
> On Mon, Jul 22, 2019 at 12:54:51PM +0300, Dmitry Osipenko wrote:
>>
>> All Tegra SoCs support SC7, hence the 'supports_sc7' and the comment
>> doesn't sound correct to me. Something like 'firmware_sc7' should suit
>> better here.
>>
>>> + writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
>>
>> Secondly, I'm also not sure why COP interrupts need to be disabled for
>> pre-T210 at all, since COP is unused. This looks to me like it was
>> cut-n-pasted from downstream kernel without a good reason and could be
>> simply removed.
>
> I don't think we can rely on the fact that COP is unused. People can
> write their own code to run on COP.
1. Not upstream - doesn't matter.
2. That's not very good if something unknown is running on COP and then
kernel suddenly intervenes, don't you think so?
^ permalink raw reply
* Re: [PATCH v3 1/5] devfreq: exynos-bus: correct clock enable sequence
From: Chanwoo Choi @ 2019-07-25 9:58 UTC (permalink / raw)
To: k.konieczny
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Krzysztof Kozlowski,
Kukjin Kim, Kyungmin Park, Mark Rutland, MyungJoo Ham,
Nishanth Menon, Rob Herring, Stephen Boyd, Viresh Kumar,
devicetree, linux-arm-kernel, linux-kernel, linux-pm,
linux-samsung-soc
In-Reply-To: <20190719150535.15501-2-k.konieczny@partner.samsung.com>
Hi Kamil,
On 19. 7. 20. 오전 12:05, k.konieczny@partner.samsung.com wrote:
> Regulators should be enabled before clocks to avoid h/w hang. This
> require change in exynos_bus_probe() to move exynos_bus_parse_of()
> after exynos_bus_parent_parse_of() and change in enabling sequence
> of regulator and clock in exynos_bus_parse_of(). Similar change is
> needed in exynos_bus_exit() where clock should be disabled first.
>
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
> This patch is new to this series.
>
> ---
> drivers/devfreq/exynos-bus.c | 58 ++++++++++++++++++++----------------
> 1 file changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 486cc5b422f1..f391044aa39d 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -194,11 +194,11 @@ static void exynos_bus_exit(struct device *dev)
> if (ret < 0)
> dev_warn(dev, "failed to disable the devfreq-event devices\n");
>
> + clk_disable_unprepare(bus->clk);
> if (bus->regulator)
> regulator_disable(bus->regulator);
>
> dev_pm_opp_of_remove_table(dev);
> - clk_disable_unprepare(bus->clk);
> }
>
> /*
> @@ -326,8 +326,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
> return ret;
> }
>
> -static int exynos_bus_parse_of(struct device_node *np,
> - struct exynos_bus *bus)
> +static int exynos_bus_parse_of(struct exynos_bus *bus)
> {
> struct device *dev = bus->dev;
> struct dev_pm_opp *opp;
> @@ -341,36 +340,35 @@ static int exynos_bus_parse_of(struct device_node *np,
> return PTR_ERR(bus->clk);
> }
>
> - ret = clk_prepare_enable(bus->clk);
> + /* Get the freq and voltage from OPP table to scale the bus freq */
> + ret = dev_pm_opp_of_add_table(dev);
> if (ret < 0) {
> - dev_err(dev, "failed to get enable clock\n");
> + dev_err(dev, "failed to get OPP table\n");
> return ret;
> }
>
> - /* Get the freq and voltage from OPP table to scale the bus freq */
> - ret = dev_pm_opp_of_add_table(dev);
> + ret = clk_prepare_enable(bus->clk);
> if (ret < 0) {
> - dev_err(dev, "failed to get OPP table\n");
> + dev_err(dev, "failed to enable clock\n");
> goto err_clk;
> }
> -
> rate = clk_get_rate(bus->clk);
>
> opp = devfreq_recommended_opp(dev, &rate, 0);
> if (IS_ERR(opp)) {
> dev_err(dev, "failed to find dev_pm_opp\n");
> ret = PTR_ERR(opp);
> - goto err_opp;
> + goto err;
> }
> bus->curr_freq = dev_pm_opp_get_freq(opp);
> dev_pm_opp_put(opp);
>
> return 0;
>
> -err_opp:
> - dev_pm_opp_of_remove_table(dev);
> -err_clk:
> +err:
> clk_disable_unprepare(bus->clk);
> +err_clk:
> + dev_pm_opp_of_remove_table(dev);
>
> return ret;
> }
> @@ -386,6 +384,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
> struct exynos_bus *bus;
> int ret, max_state;
> unsigned long min_freq, max_freq;
> + bool passive = false;
>
> if (!np) {
> dev_err(dev, "failed to find devicetree node\n");
> @@ -399,27 +398,31 @@ static int exynos_bus_probe(struct platform_device *pdev)
> bus->dev = &pdev->dev;
> platform_set_drvdata(pdev, bus);
>
> - /* Parse the device-tree to get the resource information */
> - ret = exynos_bus_parse_of(np, bus);
> - if (ret < 0)
> - return ret;
> -
> profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
> - if (!profile) {
> - ret = -ENOMEM;
> - goto err;
> - }
> + if (!profile)
> + return -ENOMEM;
>
> node = of_parse_phandle(dev->of_node, "devfreq", 0);
> if (node) {
> of_node_put(node);
> - goto passive;
> + passive = true;
> } else {
> ret = exynos_bus_parent_parse_of(np, bus);
> + if (ret < 0)
> + return ret;
> }
>
> - if (ret < 0)
> - goto err;
> + /* Parse the device-tree to get the resource information */
> + ret = exynos_bus_parse_of(bus);
> + if (ret < 0) {
> + if (!passive)
> + regulator_disable(bus->regulator);
> +
> + return ret;
> + }
> +
> + if (passive)
> + goto passive;
>
> /* Initialize the struct profile and governor data for parent device */
> profile->polling_ms = 50;
> @@ -508,8 +511,11 @@ static int exynos_bus_probe(struct platform_device *pdev)
> return 0;
>
> err:
> - dev_pm_opp_of_remove_table(dev);
> clk_disable_unprepare(bus->clk);
> + if (!passive)
> + regulator_disable(bus->regulator);
> +
> + dev_pm_opp_of_remove_table(dev);
>
> return ret;
> }
>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* Re: [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
From: Peter De Schrijver @ 2019-07-25 9:55 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland, pgaikwad,
sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <f6582e43-168e-1b7e-9db8-3d263bc3ba0d@gmail.com>
On Mon, Jul 22, 2019 at 12:54:51PM +0300, Dmitry Osipenko wrote:
>
> All Tegra SoCs support SC7, hence the 'supports_sc7' and the comment
> doesn't sound correct to me. Something like 'firmware_sc7' should suit
> better here.
>
> > + writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
>
> Secondly, I'm also not sure why COP interrupts need to be disabled for
> pre-T210 at all, since COP is unused. This looks to me like it was
> cut-n-pasted from downstream kernel without a good reason and could be
> simply removed.
I don't think we can rely on the fact that COP is unused. People can
write their own code to run on COP.
Peter.
^ permalink raw reply
* [PATCH 3/3] regulator: twl6030: workaround the VMMC reset behavior
From: Gregory CLEMENT @ 2019-07-25 9:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, linux-kernel
Cc: Rob Herring, devicetree, Tony Lindgren, linux-omap,
Thomas Petazzoni, Gregory CLEMENT
In-Reply-To: <20190725094542.16547-1-gregory.clement@bootlin.com>
During reset the VMMC regulator doesn't reach 0V and only drops to
1.8V, furthermore the pulse width is under 200us whereas the SD
specification expect 1ms.
The WR_S bit allows the TWL6030 to no reset at all the VMMC during warm
reset and keep the current voltage. Thanks to this workaround the SD
card doesn't reach a undefined reset stage.
Actually this behavior is available for all the LDO regulator, so the
driver will also allow to use it with any of these regulator.
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
drivers/regulator/twl6030-regulator.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/twl6030-regulator.c b/drivers/regulator/twl6030-regulator.c
index d73c81542ceb..b8100c3cedad 100644
--- a/drivers/regulator/twl6030-regulator.c
+++ b/drivers/regulator/twl6030-regulator.c
@@ -57,6 +57,9 @@ struct twlreg_info {
#define VREG_BC_PROC 3
#define VREG_BC_CLK_RST 4
+/* TWL6030 LDO register values for VREG_VOLTAGE */
+#define TWL6030_VREG_VOLTAGE_WR_S BIT(7)
+
/* TWL6030 LDO register values for CFG_STATE */
#define TWL6030_CFG_STATE_OFF 0x00
#define TWL6030_CFG_STATE_ON 0x01
@@ -68,9 +71,10 @@ struct twlreg_info {
#define TWL6030_CFG_STATE_APP(v) (((v) & TWL6030_CFG_STATE_APP_MASK) >>\
TWL6030_CFG_STATE_APP_SHIFT)
-/* Flags for SMPS Voltage reading */
+/* Flags for SMPS Voltage reading and LDO reading*/
#define SMPS_OFFSET_EN BIT(0)
#define SMPS_EXTENDED_EN BIT(1)
+#define TWL_6030_WARM_RESET BIT(3)
/* twl6032 SMPS EPROM values */
#define TWL6030_SMPS_OFFSET 0xB0
@@ -250,6 +254,9 @@ twl6030ldo_set_voltage_sel(struct regulator_dev *rdev, unsigned selector)
{
struct twlreg_info *info = rdev_get_drvdata(rdev);
+ if (info->flags & TWL_6030_WARM_RESET)
+ selector |= TWL6030_VREG_VOLTAGE_WR_S;
+
return twlreg_write(info, TWL_MODULE_PM_RECEIVER, VREG_VOLTAGE,
selector);
}
@@ -259,6 +266,9 @@ static int twl6030ldo_get_voltage_sel(struct regulator_dev *rdev)
struct twlreg_info *info = rdev_get_drvdata(rdev);
int vsel = twlreg_read(info, TWL_MODULE_PM_RECEIVER, VREG_VOLTAGE);
+ if (info->flags & TWL_6030_WARM_RESET)
+ vsel &= ~TWL6030_VREG_VOLTAGE_WR_S;
+
return vsel;
}
@@ -710,6 +720,9 @@ static int twlreg_probe(struct platform_device *pdev)
break;
}
+ if (of_get_property(np, "ti,retain-on-reset", NULL))
+ info->flags |= TWL_6030_WARM_RESET;
+
config.dev = &pdev->dev;
config.init_data = initdata;
config.driver_data = info;
--
2.20.1
^ permalink raw reply related
* [PATCH 2/3] regulator: twl6030: use variable for device node
From: Gregory CLEMENT @ 2019-07-25 9:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, linux-kernel
Cc: Rob Herring, devicetree, Tony Lindgren, linux-omap,
Thomas Petazzoni, Gregory CLEMENT
In-Reply-To: <20190725094542.16547-1-gregory.clement@bootlin.com>
Instead of refering the full pdev->dev.of_node use a local variable.
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
drivers/regulator/twl6030-regulator.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/regulator/twl6030-regulator.c b/drivers/regulator/twl6030-regulator.c
index 5fe208b381eb..d73c81542ceb 100644
--- a/drivers/regulator/twl6030-regulator.c
+++ b/drivers/regulator/twl6030-regulator.c
@@ -665,14 +665,14 @@ static int twlreg_probe(struct platform_device *pdev)
struct regulation_constraints *c;
struct regulator_dev *rdev;
struct regulator_config config = { };
+ struct device_node *np = pdev->dev.of_node;
template = of_device_get_match_data(&pdev->dev);
if (!template)
return -ENODEV;
id = template->desc.id;
- initdata = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
- &template->desc);
+ initdata = of_get_regulator_init_data(&pdev->dev, np, &template->desc);
if (!initdata)
return -EINVAL;
@@ -713,7 +713,7 @@ static int twlreg_probe(struct platform_device *pdev)
config.dev = &pdev->dev;
config.init_data = initdata;
config.driver_data = info;
- config.of_node = pdev->dev.of_node;
+ config.of_node = np;
rdev = devm_regulator_register(&pdev->dev, &info->desc, &config);
if (IS_ERR(rdev)) {
--
2.20.1
^ permalink raw reply related
* [PATCH 1/3] dt-bindings: regulator: twl6030: Add retain-on-reset property
From: Gregory CLEMENT @ 2019-07-25 9:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, linux-kernel
Cc: Rob Herring, devicetree, Tony Lindgren, linux-omap,
Thomas Petazzoni, Gregory CLEMENT
In-Reply-To: <20190725094542.16547-1-gregory.clement@bootlin.com>
During reset the VMMC regulator doesn't reach 0V and only drops to
1.8V, furthermore the pulse width is under 200us whereas the SD
specification expect 1ms.
For this 2 reasons being able to no reset at all the VMMC during warm
reset and keep the current voltage is a good workaround. The TWL6030
allows this but needs to be aware of it and this configuration should
also be shared with the bootloader.
This is the purpose of this new property: ti,retain-on-reset
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
.../devicetree/bindings/regulator/twl-regulator.txt | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/twl-regulator.txt b/Documentation/devicetree/bindings/regulator/twl-regulator.txt
index 74a91c4f8530..549f80436deb 100644
--- a/Documentation/devicetree/bindings/regulator/twl-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/twl-regulator.txt
@@ -71,3 +71,10 @@ Example:
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <3000000>;
};
+
+For twl6030 regulators/LDOs:
+
+ - ti,retain-on-reset: Does not turn off the supplies during warm
+ reset. Could be needed for VMMC, as TWL6030
+ reset sequence for this signal does not comply
+ with the SD specification.
--
2.20.1
^ permalink raw reply related
* [PATCH 0/3] regulator: twl6030: Fix the VMMC reset behavior
From: Gregory CLEMENT @ 2019-07-25 9:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, linux-kernel
Cc: Rob Herring, devicetree, Tony Lindgren, linux-omap,
Thomas Petazzoni, Gregory CLEMENT
Hello,
With the TWL6030 PMIC, during reset the VMMC regulator doesn't reach
0V and only drops to 1.8V, furthermore the pulse width is under 200us
whereas the SD specification expect 1ms.
Fortunately, the WR_S bit allows the TWL6030 to no reset at all the
VMMC during warm reset and keep the current voltage. Thanks to this
workaround the SD card doesn't reach a undefined reset stage.
The first patch describes the new property needed for this "feature".
The second one is just a small cleanup done while I wrote the last
patch, but as it was not really related to the feature itself, I made
a separate patch for it.
The last patch adds the feature in the driver.
Gregory
Gregory CLEMENT (3):
dt-bindings: regulator: twl6030: Add retain-on-reset property
regulator: twl6030: use variable for device node
regulator: twl6030: workaround the VMMC reset behavior
.../bindings/regulator/twl-regulator.txt | 7 +++++++
drivers/regulator/twl6030-regulator.c | 21 +++++++++++++++----
2 files changed, 24 insertions(+), 4 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH] ARM: dts: stm32: add audio codec support on stm32mp157a-dk1 board
From: Olivier MOYSAN @ 2019-07-25 9:41 UTC (permalink / raw)
To: Alexandre TORGUE, linux-stm32@st-md-mailman.stormreply.com,
robh@kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <27476214-07fe-886b-1cab-20902837f29c@st.com>
On 7/24/19 6:40 PM, Alexandre Torgue wrote:
> Hi Olivier
>
> On 7/5/19 1:53 PM, Olivier Moysan wrote:
>> Add support of Cirrus cs42l51 audio codec on stm32mp157a-dk1 board.
>> Configuration overview:
>> - SAI2A is the CPU interface used for the codec audio playback
>> - SAI2B is the CPU interface used for the codec audio record
>> - SAI2A is configured as a clock provider for the audio codec
>> - SAI2A&B are configured as slave of the audio codec
>> - SAI2A&B share the same interface of the audio codec
>>
>> Note:
>> In master mode, cs42l51 audio codec provides a bitclock
>> at 64 x FS, regardless of data width. This means that
>> slot width is always 32 bits.
>> Set slot width to 32 bits and slot number to 2
>> in SAI2A&B endpoint nodes, to match this constraint.
>> dai-tdm-slot-num and dai-tdm-slot-width properties are used here,
>> assuming that i2s is a special case of tdm, where slot number is 2.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
>> ---
>> arch/arm/boot/dts/stm32mp157a-dk1.dts | 89 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 89 insertions(+)
>>
>
> ...
>
>>
>> +&sai2 {
>> + clocks = <&rcc SAI2>, <&rcc PLL3_Q>, <&rcc PLL3_R>;
>> + clock-names = "pclk", "x8k", "x11k";
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-0 = <&sai2a_pins_a>, <&sai2b_pins_b>;
>> + pinctrl-1 = <&sai2a_sleep_pins_a>, <&sai2b_sleep_pins_b>;
>> + status = "okay";
>> +
>> + sai2a: audio-controller@4400b004 {
>> + #clock-cells = <0>;
>> + dma-names = "tx";
>> + clocks = <&rcc SAI2_K>;
>> + clock-names = "sai_ck";
>> + status = "okay";
>> +
>> + sai2a_port: port {
>> + sai2a_endpoint: endpoint {
>> + remote-endpoint = <&cs42l51_tx_endpoint>;
>> + format = "i2s";
>> + mclk-fs = <256>;
>> + dai-tdm-slot-num = <2>;
>> + dai-tdm-slot-width = <32>;
>> + };
>> + };
>> + };
>> +
> You could use label to overload sai2a and sai2b. no ?
I propose to keep it unchanged for better readability
>
>> + sai2b: audio-controller@4400b024 {
>> + dma-names = "rx";
>> + st,sync = <&sai2a 2>;
>> + clocks = <&rcc SAI2_K>, <&sai2a>;
>> + clock-names = "sai_ck", "MCLK";
>> + status = "okay";
>> +
>> + sai2b_port: port {
>> + sai2b_endpoint: endpoint {
>> + remote-endpoint = <&cs42l51_rx_endpoint>;
>> + format = "i2s";
>> + mclk-fs = <256>;
>> + dai-tdm-slot-num = <2>;
>> + dai-tdm-slot-width = <32>;
>> + };
>> + };
>> + };
>> +};
>> +
>> &sdmmc1 {
>> pinctrl-names = "default", "opendrain", "sleep";
>> pinctrl-0 = <&sdmmc1_b4_pins_a>;
>>
^ permalink raw reply
* Re: [PATCH] ARM: dts: stm32: activate dma for qspi on stm32mp157
From: Alexandre Torgue @ 2019-07-25 9:33 UTC (permalink / raw)
To: Ludovic Barre, Rob Herring, Mark Rutland
Cc: devicetree, linux-stm32, linux-arm-kernel, Maxime Coquelin,
linux-kernel
In-Reply-To: <1561637345-31441-1-git-send-email-ludovic.Barre@st.com>
Hi Ludovic
On 6/27/19 2:09 PM, Ludovic Barre wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
>
> This patch activates dma for qspi on stm32mp157.
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
> arch/arm/boot/dts/stm32mp157c.dtsi | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 2afeee6..205ea1d 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -1074,6 +1074,9 @@
> reg = <0x58003000 0x1000>, <0x70000000 0x10000000>;
> reg-names = "qspi", "qspi_mm";
> interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
> + dmas = <&mdma1 22 0x10 0x100002 0x0 0x0>,
> + <&mdma1 22 0x10 0x100008 0x0 0x0>;
> + dma-names = "tx", "rx";
> clocks = <&rcc QSPI_K>;
> resets = <&rcc QSPI_R>;
> status = "disabled";
>
Applied on stm32-next.
Thanks.
Alex
^ permalink raw reply
* Re: [RFC,v3 7/9] media: platform: Add Mediatek ISP P1 device driver
From: Tomasz Figa @ 2019-07-25 9:23 UTC (permalink / raw)
To: Jungo Lin
Cc: devicetree, Sean Cheng (鄭昇弘),
Frederic Chen (陳俊元),
Rynn Wu (吳育恩), srv_heupstream, Rob Herring,
Ryan Yu (余孟修),
Frankie Chiu (邱文凱), Hans Verkuil, ddavenport,
Sj Huang, moderated list:ARM/Mediatek SoC support,
Laurent Pinchart, Matthias Brugger, Mauro Carvalho Chehab
In-Reply-To: <1563616681.1212.386.camel@mtksdccf07>
.Hi Jungo,
On Sat, Jul 20, 2019 at 6:58 PM Jungo Lin <jungo.lin@mediatek.com> wrote:
>
> Hi, Tomasz:
>
> On Wed, 2019-07-10 at 18:56 +0900, Tomasz Figa wrote:
> > Hi Jungo,
> >
> > On Tue, Jun 11, 2019 at 11:53:42AM +0800, Jungo Lin wrote:
> > > This patch adds the Mediatek ISP P1 HW control device driver.
> > > It handles the ISP HW configuration, provides interrupt handling and
> > > initializes the V4L2 device nodes and other functions.
> > >
> > > (The current metadata interface used in meta input and partial
> > > meta nodes is only a temporary solution to kick off the driver
> > > development and is not ready to be reviewed yet.)
> > >
> > > Signed-off-by: Jungo Lin <jungo.lin@mediatek.com>
> > > ---
> > > .../platform/mtk-isp/isp_50/cam/Makefile | 1 +
> > > .../mtk-isp/isp_50/cam/mtk_cam-regs.h | 126 ++
> > > .../platform/mtk-isp/isp_50/cam/mtk_cam.c | 1087 +++++++++++++++++
> > > .../platform/mtk-isp/isp_50/cam/mtk_cam.h | 243 ++++
> > > 4 files changed, 1457 insertions(+)
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-regs.h
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.c
> > > create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.h
> > >
> >
> > Thanks for the patch! Please see my comments inline.
> >
> > [snip]
> >
>
> Thanks for your comments. Please check my replies inline.
>
Thanks! I'll snip anything I don't have further comments on.
[snip]
> > > +/* META */
> > > +#define REG_META0_VB2_INDEX 0x14dc
> > > +#define REG_META1_VB2_INDEX 0x151c
> >
> > I don't believe these registers are really for VB2 indexes.
> >
>
> MTK P1 ISP HW supports frame header spare registers for each DMA, such
> as CAM_DMA_FH_AAO_SPARE or CAM_DMA_FH_AFO_SPARE. We could save some
> frame information in these ISP registers. In this case, we save META0
> VB2 index into AAO FH spare register and META1 VB2 index into AFO FH
> spare register. These implementation is designed for non-request 3A
> DMAs. These VB2 indexes are sent in ISP_CMD_ENQUEUE_META command of
> mtk_isp_enqueue function. So we just call CAM_DMA_FH_AAO_SPARE as
> REG_META0_VB2_INDEX for easy understanding.
Unfortunately it's not a good idea to mix hardware concepts with
naming specific to the OS the driver is written for. Better to keep
the hardware naming, e.g. CAM_DMA_FH_AAO_SPARE.
> Moreover, if we only need to
> support request mode, we should remove this here.
>
> cmd_params.cmd_id = ISP_CMD_ENQUEUE_META;
> cmd_params.meta_frame.enabled_dma = dma_port;
> cmd_params.meta_frame.vb_index = buffer->vbb.vb2_buf.index;
> cmd_params.meta_frame.meta_addr.iova = buffer->daddr;
> cmd_params.meta_frame.meta_addr.scp_addr = buffer->scp_addr;
>
Okay, removing sounds good to me. Let's keep the code simple.
[snip]
> > > +
> > > + err_status = irq_status & INT_ST_MASK_CAM_ERR;
> > > +
> > > + /* Sof, done order check */
> > > + if ((irq_status & SOF_INT_ST) && (irq_status & HW_PASS1_DON_ST)) {
> > > + dev_dbg(dev, "sof_done block cnt:%d\n", isp_dev->sof_count);
> > > +
> > > + /* Notify IRQ event and enqueue frame */
> > > + irq_handle_notify_event(isp_dev, irq_status, dma_status, 0);
> > > + isp_dev->current_frame = hw_frame_num;
> >
> > What exactly is hw_frame_num? Shouldn't we assign it before notifying the
> > event?
> >
>
> This is a another spare register for frame sequence number usage.
> It comes from struct p1_frame_param:frame_seq_no which is sent by
> SCP_ISP_FRAME IPI command. We will rename this to dequeue_frame_seq_no.
> Is it a better understanding?
I'm sorry, unfortunately it's still not clear to me. Is it the
sequence number of the frame that was just processed and returned to
the kernel or the next frame that is going to be processed from now
on?
>
> Below is our frame request handling in current design.
>
> 1. Buffer preparation
> - Combined image buffers (IMGO/RRZO) + meta input buffer (Tuining) +
> other meta histogram buffers (LCSO/LMVO) into one request.
> - Accumulated one unique frame sequence number to each request and send
> this request to the SCP composer to compose CQ (Command queue) buffer
> via SCP_ISP_FRAME IPI command.
> - CQ buffer is frame registers set. If ISP registers should be updated
> per frame, these registers are configured in the CQ buffer, such as
> frame sequence number, DMA addresses and tuning ISP registers.
> - One frame request will be composed into one CQ buffer.Once CQ buffer
> is composed done and kernel driver will receive ISP_CMD_FRAME_ACK with
> its corresponding frame sequence number. Based on this, kernel driver
> knows which request is ready to be en-queued and save this with
> p1_dev->isp_ctx.composed_frame_id.
Hmm, why do we need to save this in p1_dev->isp_ctx? Wouldn't we
already have a linked lists of requests that are composed and ready to
be enqueued? Also, the request itself would contain its frame ID
inside the driver request struct, right?
> - The maximum number of CQ buffers in SCP is 3.
>
> 2. Buffer en-queue flow
> - In order to configure correct CQ buffer setting before next SQF event,
> it is depended on by MTK ISP P1 HW CQ mechanism.
> - The basic concept of CQ mechanism is loaded ISP CQ buffer settings
> when HW_PASS1_DON_ST is received which means DMA output is done.
> - Btw, the pre-condition of this, need to tell ISP HW which CQ buffer
> address is used. Otherwise, it will loaded one dummy CQ buffer to
> bypass.
> - So we will check available CQ buffers by comparing composed frame
> sequence number & dequeued frame sequence from ISP HW in SOF event.
> - If there are available CQ buffers, update the CQ base address to the
> next CQ buffer address based on current de-enqueue frame sequence
> number. So MTK ISP P1 HW will load this CQ buffer into HW when
> HW_PASS1_DON_ST is triggered which is before the next SOF.
> - So in next SOF event, ISP HW starts to output DMA buffers with this
> request until request is done.
> - But, for the first request, it is loaded into HW manually when
> streaming is on for better performance.
>
> 3. Buffer de-queue flow
> - We will use frame sequence number to decide which request is ready to
> de-queue.
> - We will save some important register setting from ISP HW when SOF is
> received. This is because the ISP HW starts to output the data with the
> corresponding settings, especially frame sequence number setting.
Could you explain a bit more about these important register settings?
When does the hardware update the values in the register to new ones?
At SOF?
> - When receiving SW_PASS1_DON_ST IRQ event, it means the DMA output is
> done. So we could call isp_deque_request_frame with frame sequence
> number to de-queue frame to VB2
What's the difference between HW_PASS1_DON_ST and SW_PASS1_DON_ST?
> - For AAO/AFO buffers, it has similar design concept. Sometimes, if only
> AAO/AFO non-request buffers are en-queued without request buffers at the
> same time, there will be no SW P1 done event for AAO/AFO DMA done.
> Needs to depend on other IRQ events, such as AAO/AFO_DONE_EVENT.
Do we have a case like this? Wouldn't we normally always want to
bundle AAO/AFO buffers with frame buffers?
> - Due to CQ buffer number limitation, if we receive SW_PASS1_DONT_ST,
> we may try to send another request to SCP for composing.
Okay, so basically in SW_PASS1_DONT_ST the CQ completed reading the CQ
buffers, right?
>
> Hopefully, my explanation is helpful for better understanding our
> implementation. If you still have any questions, please let me know.
>
Yes, it's more clear now, thanks. Still some more comments above, though.
> > > + isp_dev->meta0_vb2_index = meta0_vb2_index;
> > > + isp_dev->meta1_vb2_index = meta1_vb2_index;
> > > + } else {
> > > + if (irq_status & SOF_INT_ST) {
> > > + isp_dev->current_frame = hw_frame_num;
> > > + isp_dev->meta0_vb2_index = meta0_vb2_index;
> > > + isp_dev->meta1_vb2_index = meta1_vb2_index;
> > > + }
> > > + irq_handle_notify_event(isp_dev, irq_status, dma_status, 1);
> > > + }
> >
> > The if and else blocks do almost the same things just in different order. Is
> > it really expected?
> >
>
> If we receive HW_PASS1_DON_ST & SOF_INT_ST IRQ events at the same time,
> the correct sequence should be handle HW_PASS1_DON_ST firstly to check
> any de-queued frame and update the next frame setting later.
> Normally, this is a corner case or system performance issue.
So it sounds like HW_PASS1_DON_ST means that all data from current
frame has been written, right? If I understand your explanation above
correctly, that would mean following handling of each interrupt:
HW_PASS1_DON_ST:
- CQ executes with next CQ buffer to prepare for next frame. <- how
is this handled? does the CQ hardware automatically receive this event
from the ISP hadware?
- return VB2 buffers,
- complete requests.
SOF_INT_ST:
- send VSYNC event to userspace,
- program next CQ buffer to CQ,
SW_PASS1_DON_ST:
- reclaim CQ buffer and enqueue next frame to composing if available
>
> Btw, we will revise the above implementation as below.
>
>
> if (irq_status & SOF_INT_ST)
> mtk_cam_dev_event_frame_sync(&p1_dev->cam_dev,
> dequeue_frame_seq_no);
>
> /* Sof, done order check */
> if ((irq_status & SOF_INT_ST) && (irq_status & HW_PASS1_DON_ST))
> dev_warn(dev, "sof_done block cnt:%d\n", p1_dev->sof_count);
>
> /* Notify IRQ event and de-enqueue frame */
> irq_handle_notify_event(p1_dev, irq_status, dma_status);
Don't we still need to do this conditionally, only if we got HW_PASS1_DON_ST?
[snip]
> > > +/* ISP P1 interface functions */
> > > +int mtk_isp_power_init(struct mtk_cam_dev *cam_dev)
> > > +{
> > > + struct device *dev = &cam_dev->pdev->dev;
> > > + struct isp_p1_device *p1_dev = get_p1_device(dev);
> > > + struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> > > + int ret;
> > > +
> > > + ret = isp_setup_scp_rproc(p1_dev);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = isp_init_context(p1_dev);
> > > + if (ret)
> > > + return ret;
> >
> > The above function doesn't really seem to be related to power management.
> > Should it be called from subdev stream on?
> >
>
> We will rename this function to mtk_isp_hw_init.
> But, it will be called when the first video node is streamed on.
> This is because we need to initialize the HW firstly for sub-device
> stream-on performance. We need to send some IPI commands, such as
> ISP_CMD_INIT & ISP_CMD_CONFIG_META & ISP_CMD_ENQUEUE_META in this
> timing.
What performance do you mean here? The time between first video node
stream on and last video node stream on should be really short. Are
you seeing some long delays there?
That said, doing it when the first video node starts streaming is okay.
[snip]
> > > + /* Use pure RAW as default HW path */
> > > + isp_ctx->isp_raw_path = ISP_PURE_RAW_PATH;
> > > + atomic_set(&p1_dev->cam_dev.streamed_node_count, 0);
> > > +
> > > + isp_composer_hw_init(dev);
> > > + /* Check enabled DMAs which is configured by media setup */
> > > + isp_composer_meta_config(dev, get_enabled_dma_ports(cam_dev));
> >
> > Hmm, this seems to be also configured by isp_compoer_hw_config(). Are both
> > necessary?
> >
>
> Yes, it is necessary for non-request buffers design for Camera 3A video
> nodes. For 3A video nodes, we just want to know which 3A video nodes are
> enabled in ISP_CMD_CONFIG_META. In this stage, we may not know the image
> format from user space. So we just pass the enabled DMA information from
> kernel to SCP only. With 3A enabled DMA information, we could configure
> related 3A registers in SCP.
We should try to remove this non-request mode. Let's continue
discussion on the other patch where I brought this topic.
[snip]
> > > +int mtk_isp_power_release(struct device *dev)
> > > +{
> > > + isp_composer_hw_deinit(dev);
> > > + isp_uninit_context(dev);
> >
> > These two don't seem to be related to power either.
> >
> > Instead, I don't see anything that could undo the rproc_boot() operation
> > here.
> >
>
> We will rename this function to mtk_isp_hw_release.
> We will also add rproc_shutdown function call here.
>
> int mtk_isp_hw_release(struct mtk_cam_dev *cam)
> {
> struct device *dev = cam->dev;
> struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(dev);
>
> isp_composer_hw_deinit(p1_dev);
> pm_runtime_put_sync_autosuspend(dev);
Note that for autosuspend to work correctly, you also need to call
pm_runtime_mark_last_busy() before this one.
[snip]
> > > + struct mtk_isp_p1_ctx *isp_ctx = &p1_dev->isp_ctx;
> > > + struct p1_frame_param frameparams;
> > > + struct mtk_isp_queue_job *framejob;
> > > + struct media_request_object *obj, *obj_safe;
> > > + struct vb2_buffer *vb;
> > > + struct mtk_cam_dev_buffer *buf;
> > > +
> > > + framejob = kzalloc(sizeof(*framejob), GFP_ATOMIC);
> >
> > This allocation shouldn't be needed. The structure should be already a part
> > of the mtk_cam_dev_request struct that wraps media_request. Actually. I'd
> > even say that the contents of this struct should be just moved to that one
> > to avoid overabstracting.
> >
>
> For this function, we will apply the new design from P2 driver's
> comment. Here is the new implementation.
>
> struct mtk_cam_dev_request {
> struct media_request req;
> struct mtk_p1_frame_param frame_params;
> struct work_struct frame_work;
> struct list_head list;
> atomic_t buf_count;
> };
>
> void mtk_isp_req_enqueue(struct mtk_cam_dev *cam,
> struct mtk_cam_dev_request *req)
> {
> struct mtk_isp_p1_device *p1_dev = dev_get_drvdata(cam->dev);
> int ret;
>
> req->frame_params.frame_seq_no = ++p1_dev->enqueue_frame_seq_no;
> INIT_WORK(&req->frame_work, isp_tx_frame_worker);
> ret = queue_work(p1_dev->composer_wq, &req->frame_work);
> if (!ret)
> dev_dbg(cam->dev, "frame_no:%d queue_work failed\n",
> req->frame_params.frame_seq_no, ret);
> else
> dev_dbg(cam->dev, "Enqueue fd:%s frame_seq_no:%d job cnt:%d\n",
> req->req.debug_str, req->frame_params.frame_seq_no,
> atomic_read(&cam->running_job_count));
It shouldn't be possible for queue_work() to fail here. We just
received a brand new request from the Request API core and called
INIT_WORK() on the work struct.
[snip]
> > > + enable_sys_clock(p1_dev);
> > > +
> > > + /* V4L2 stream-on phase & restore HW stream-on status */
> > > + if (p1_dev->cam_dev.streaming) {
> > > + dev_dbg(dev, "Cam:%d resume,enable VF\n", module);
> > > + /* Enable CMOS */
> > > + reg_val = readl(isp_dev->regs + REG_TG_SEN_MODE);
> > > + writel((reg_val | CMOS_EN_BIT),
> > > + isp_dev->regs + REG_TG_SEN_MODE);
> > > + /* Enable VF */
> > > + reg_val = readl(isp_dev->regs + REG_TG_VF_CON);
> > > + writel((reg_val | VFDATA_EN_BIT),
> > > + isp_dev->regs + REG_TG_VF_CON);
> > > + }
> >
> > Does the hardware keep all the state, e.g. queued buffers, during suspend?
> > Would the code above continue all the capture from the next buffer, as
> > queued by the userspace before the suspend?
> >
>
> Yes, we will test it.
> 1. SCP buffers are kept by SCP processor
> 2. ISP registers are still kept even if ISP clock is disable.
>
That said, during system suspend, it would be more than just ISP clock
disabled. I'd expect that the ISP power domain would be powered off.
However, if we ensure that the ISP completes before suspend, I guess
that after the resume the next frame CQ buffer would reprogram all the
registers, right?
Also, would SCP always keep running in system suspend?
[snip]
> > > +
> > > + for (i = ISP_CAMSYS_CONFIG_IDX; i < ISP_DEV_NODE_NUM; i++) {
> >
> > I think we want to start from 0 here?
> >
>
> Because of single CAM support, we will revise our DTS tree to support
> single CAM only.
Note that DT bindings should describe the hardware not the driver. So
please design the bindings in a way that would cover all the cameras,
even if the driver only takes the information needed to handle 1.
> So we could remove this loop and check the CAM-B HW
> information here. Here is below new function.
>
> static int mtk_isp_probe(struct platform_device *pdev)
> {
> /* List of clocks required by isp cam */
> static const char * const clk_names[] = {
> "camsys_cam_cgpdn", "camsys_camtg_cgpdn"
> };
> struct mtk_isp_p1_device *p1_dev;
> struct device *dev = &pdev->dev;
> struct resource *res;
> int irq, ret, i;
>
> p1_dev = devm_kzalloc(dev, sizeof(*p1_dev), GFP_KERNEL);
> if (!p1_dev)
> return -ENOMEM;
>
> p1_dev->dev = dev;
> dev_set_drvdata(dev, p1_dev);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> p1_dev->regs = devm_ioremap_resource(dev, res);
> if (IS_ERR(p1_dev->regs)) {
> dev_err(dev, "Failed platform resources map\n");
> return PTR_ERR(p1_dev->regs);
> }
> dev_dbg(dev, "cam, map_addr=0x%pK\n", p1_dev->regs);
>
> irq = platform_get_irq(pdev, 0);
> if (!irq) {
> dev_err(dev, "Missing IRQ resources data\n");
> return -ENODEV;
> }
> ret = devm_request_irq(dev, irq, isp_irq_cam, IRQF_SHARED,
> dev_name(dev), p1_dev);
> if (ret) {
> dev_err(dev, "req_irq fail, dev:%s irq=%d\n",
> dev->of_node->name, irq);
> return ret;
> }
> dev_dbg(dev, "Reg. irq=%d, isr:%s\n", irq, dev_driver_string(dev));
> spin_lock_init(&p1_dev->spinlock_irq);
>
> p1_dev->num_clks = ARRAY_SIZE(clk_names);
> p1_dev->clks = devm_kcalloc(dev, p1_dev->num_clks,
> sizeof(*p1_dev->clks), GFP_KERNEL);
> if (!p1_dev->clks)
> return -ENOMEM;
>
> for (i = 0; i < p1_dev->num_clks; ++i)
> p1_dev->clks[i].id = clk_names[i];
>
> ret = devm_clk_bulk_get(dev, p1_dev->num_clks, p1_dev->clks);
> if (ret) {
> dev_err(dev, "cannot get isp cam clock:%d\n", ret);
> return ret;
> }
>
> ret = isp_setup_scp_rproc(p1_dev, pdev);
> if (ret)
> return ret;
>
> pm_runtime_enable(dev);
We also need to call pm_runtime_use_autosuspend() and
pm_runtime_set_autosuspend_delay() before enabling runtime PM. I'd
suggest an autosuspend delay equal to around 2x the time that's needed
to stop and start streaming in total.
[snip]
> > > +static const struct dev_pm_ops mtk_isp_pm_ops = {
> > > + SET_SYSTEM_SLEEP_PM_OPS(mtk_isp_suspend, mtk_isp_resume)
> > > + SET_RUNTIME_PM_OPS(mtk_isp_suspend, mtk_isp_resume, NULL)
> >
> > For V4L2 drivers system and runtime PM ops would normally be completely
> > different. Runtime PM ops would be called when the hardware is idle already
> > or is about to become active. System PM ops would be called at system power
> > state change and the hardware might be both idle or active. Please also see
> > my comments to mtk_isp_suspend() and mtk_isp_resume() above.
> >
>
> Here is the new implementation. It should be clear to show the
> difference between system and runtime PM ops.
>
> static const struct dev_pm_ops mtk_isp_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> pm_runtime_force_resume)
> SET_RUNTIME_PM_OPS(mtk_isp_runtime_suspend, mtk_isp_runtime_resume,
> NULL)
> };
That's still not correct. In runtime suspend/resume ops we already are
not streaming anymore, because we call pm_runtime_get/put_*() when
starting and stopping streaming. In system suspend/resume ops we might
be streaming and that's when we need to stop the hardware and wait for
it to finish. Please implement these ops separately.
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH v2 2/2] dt-bindings: usb: renesas_gen3: Rename bindings documentation file
From: Greg Kroah-Hartman @ 2019-07-25 9:09 UTC (permalink / raw)
To: Simon Horman
Cc: Geert Uytterhoeven, Yoshihiro Shimoda, Kuninori Morimoto,
Magnus Damm, USB list, Linux-Renesas, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Niklas Söderlund
In-Reply-To: <20190711080302.qeotxueyvlr7pvhe@verge.net.au>
On Thu, Jul 11, 2019 at 10:03:03AM +0200, Simon Horman wrote:
> On Wed, Jul 03, 2019 at 02:28:51PM +0200, Geert Uytterhoeven wrote:
> > Hi Simon,
> >
> > On Wed, Jul 3, 2019 at 10:35 AM Simon Horman <horms+renesas@verge.net.au> wrote:
> > > For consistency with the naming of (most) other documentation files for DT
> > > bindings for Renesas IP blocks rename the Renesas USB3.0 peripheral
> > > documentation file from renesas-gen3.txt to renesas,usb3-peri.txt
> > >
> > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > Reviewed-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
> > >
> > > ---
> > > v2
> > > * Accumulate review tags
> > > * Use renesas,usb3-peri.txt as new filename as suggested by Shimoda-san
> >
> > Unfortunately the previous version has already made it into usb-next
> > 23c46801d14cb647 dt-bindings: usb: renesas_gen3: Rename bindings
> > documentation file
>
> Ok, I guess we should go with that version.
So can you resend this series based on 5.3-rc1 so I know what to apply?
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC PATCH v2 0/4] Input: mpr121-polled: Add polled driver for MPR121
From: Dmitry Torokhov @ 2019-07-25 8:57 UTC (permalink / raw)
To: Michal Vokáč
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
linux-input, devicetree, linux-kernel, Pengutronix Kernel Team
In-Reply-To: <ef172b24-cd27-5bb0-d8b1-718f835d0647@ysoft.com>
Hi Michal,
On Tue, May 21, 2019 at 08:51:17AM +0200, Michal Vokáč wrote:
> On 21. 05. 19 7:37, Dmitry Torokhov wrote:
> > Hi Michal,
> >
> > On Fri, May 17, 2019 at 03:12:49PM +0200, Michal Vokáč wrote:
> > > Hi,
> > >
> > > I have to deal with a situation where we have a custom i.MX6 based
> > > platform in production that uses the MPR121 touchkey controller.
> > > Unfortunately the chip is connected using only the I2C interface.
> > > The interrupt line is not used. Back in 2015 (Linux v3.14), my
> > > colleague modded the existing mpr121_touchkey.c driver to use polling
> > > instead of interrupt.
> > >
> > > For quite some time yet I am in a process of updating the product from
> > > the ancient Freescale v3.14 kernel to the latest mainline and pushing
> > > any needed changes upstream. The DT files for our imx6dl-yapp4 platform
> > > already made it into v5.1-rc.
> > >
> > > I rebased and updated our mpr121 patch to the latest mainline.
> > > It is created as a separate driver, similarly to gpio_keys_polled.
> > >
> > > The I2C device is quite susceptible to ESD. An ESD test quite often
> > > causes reset of the chip or some register randomly changes its value.
> > > The [PATCH 3/4] adds a write-through register cache. With the cache
> > > this state can be detected and the device can be re-initialied.
> > >
> > > The main question is: Is there any chance that such a polled driver
> > > could be accepted? Is it correct to implement it as a separate driver
> > > or should it be done as an option in the existing driver? I can not
> > > really imagine how I would do that though..
> > >
> > > There are also certain worries that the MPR121 chip may no longer be
> > > available in nonspecifically distant future. In case of EOL I will need
> > > to add a polled driver for an other touchkey chip. May it be already
> > > in mainline or a completely new one.
> >
> > I think that my addition of input_polled_dev was ultimately a wrong
> > thing to do. I am looking into enabling polling mode for regular input
> > devices as we then can enable polling mode in existing drivers.
>
> OK, that sounds good. Especially when one needs to switch from one chip
> to another that is already in tree, the need for a whole new polling
> driver is eliminated.
Could you please try the patch below and see if it works for your use
case? Note that I have not tried running it, but it compiles so it must
be good ;)
Thanks!
Input: add support for polling to input devices
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Separating "normal" and "polled" input devices was a mistake, as often we want
to allow the very same device work on both interrupt-driven and polled mode,
depending on the board on which the device is used.
This introduces new APIs:
- input_setup_polling
- input_set_poll_interval
- input_set_min_poll_interval
- input_set_max_poll_interval
These new APIs allow switching an input device into polled mode with sysfs
attributes matching drivers using input_polled_dev APIs that will be eventually
removed.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/Makefile | 2
drivers/input/input-poller.c | 207 ++++++++++++++++++++++++++++++++++++++++++
drivers/input/input.c | 36 ++++++-
include/linux/input.h | 10 ++
4 files changed, 247 insertions(+), 8 deletions(-)
create mode 100644 drivers/input/input-poller.c
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index 40de6a7be641..e35650930371 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -6,7 +6,7 @@
# Each configuration option enables a list of files.
obj-$(CONFIG_INPUT) += input-core.o
-input-core-y := input.o input-compat.o input-mt.o ff-core.o
+input-core-y := input.o input-compat.o input-mt.o input-poller.o ff-core.o
obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o
diff --git a/drivers/input/input-poller.c b/drivers/input/input-poller.c
new file mode 100644
index 000000000000..008d6f362d60
--- /dev/null
+++ b/drivers/input/input-poller.c
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Support for polling mode for input devices.
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/jiffies.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+#include "input-poller.h"
+
+struct input_dev_poller {
+ void (*poll)(struct input_dev *dev);
+
+ unsigned int poll_interval; /* msec */
+ unsigned int poll_interval_max; /* msec */
+ unsigned int poll_interval_min; /* msec */
+
+ struct input_dev *input;
+ struct delayed_work work;
+};
+
+static void input_dev_poller_queue_work(struct input_dev_poller *poller)
+{
+ unsigned long delay;
+
+ delay = msecs_to_jiffies(poller->poll_interval);
+ if (delay >= HZ)
+ delay = round_jiffies_relative(delay);
+
+ queue_delayed_work(system_freezable_wq, &poller->work, delay);
+}
+
+static void input_dev_poller_work(struct work_struct *work)
+{
+ struct input_dev_poller *poller =
+ container_of(work, struct input_dev_poller, work.work);
+
+ poller->poll(poller->input);
+ input_dev_poller_queue_work(poller);
+}
+
+void input_dev_poller_finalize(struct input_dev_poller *poller)
+{
+ if (!poller->poll_interval)
+ poller->poll_interval = 500;
+ if (!poller->poll_interval_max)
+ poller->poll_interval_max = poller->poll_interval;
+}
+
+void input_dev_poller_start(struct input_dev_poller *poller)
+{
+ /* Only start polling if polling is enabled */
+ if (poller->poll_interval > 0) {
+ poller->poll(poller->input);
+ input_dev_poller_queue_work(poller);
+ }
+}
+
+void input_dev_poller_stop(struct input_dev_poller *poller)
+{
+ cancel_delayed_work_sync(&poller->work);
+}
+
+int input_setup_polling(struct input_dev *dev,
+ void (*poll_fn)(struct input_dev *dev))
+{
+ struct input_dev_poller *poller;
+
+ poller = kzalloc(sizeof(*poller), GFP_KERNEL);
+ if (!poller) {
+ dev_err(dev->dev.parent ?: &dev->dev,
+ "%s: unable to allocate poller structure\n", __func__);
+ return -ENOMEM;
+ }
+
+ INIT_DELAYED_WORK(&poller->work, input_dev_poller_work);
+ poller->poll = poll_fn;
+
+ dev->poller = poller;
+ return 0;
+}
+EXPORT_SYMBOL(input_setup_polling);
+
+static bool input_dev_ensure_poller(struct input_dev *dev)
+{
+ if (!dev->poller) {
+ dev_err(dev->dev.parent ?: &dev->dev,
+ "poller structure has not been set up\n");
+ return false;
+ }
+
+ return true;
+}
+
+void input_set_poll_interval(struct input_dev *dev, unsigned int interval)
+{
+ if (input_dev_ensure_poller(dev))
+ dev->poller->poll_interval = interval;
+}
+EXPORT_SYMBOL(input_set_poll_interval);
+
+void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval)
+{
+ if (input_dev_ensure_poller(dev))
+ dev->poller->poll_interval_min = interval;
+}
+EXPORT_SYMBOL(input_set_min_poll_interval);
+
+void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval)
+{
+ if (input_dev_ensure_poller(dev))
+ dev->poller->poll_interval_max = interval;
+}
+EXPORT_SYMBOL(input_set_max_poll_interval);
+
+/* SYSFS interface */
+
+static ssize_t input_dev_get_poll_interval(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct input_dev *input = to_input_dev(dev);
+
+ return sprintf(buf, "%d\n", input->poller->poll_interval);
+}
+
+static ssize_t input_dev_set_poll_interval(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct input_dev *input = to_input_dev(dev);
+ struct input_dev_poller *poller = input->poller;
+ unsigned int interval;
+ int err;
+
+ err = kstrtouint(buf, 0, &interval);
+ if (err)
+ return err;
+
+ if (interval < poller->poll_interval_min)
+ return -EINVAL;
+
+ if (interval > poller->poll_interval_max)
+ return -EINVAL;
+
+ mutex_lock(&input->mutex);
+
+ poller->poll_interval = interval;
+
+ if (input->users) {
+ cancel_delayed_work_sync(&poller->work);
+ if (poller->poll_interval > 0)
+ input_dev_poller_queue_work(poller);
+ }
+
+ mutex_unlock(&input->mutex);
+
+ return count;
+}
+
+static DEVICE_ATTR(poll, S_IRUGO | S_IWUSR,
+ input_dev_get_poll_interval, input_dev_set_poll_interval);
+
+static ssize_t input_dev_get_poll_max(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct input_dev *input = to_input_dev(dev);
+
+ return sprintf(buf, "%d\n", input->poller->poll_interval_max);
+}
+
+static DEVICE_ATTR(max, S_IRUGO, input_dev_get_poll_max, NULL);
+
+static ssize_t input_dev_get_poll_min(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct input_dev *input = to_input_dev(dev);
+
+ return sprintf(buf, "%d\n", input->poller->poll_interval_min);
+}
+
+static DEVICE_ATTR(min, S_IRUGO, input_dev_get_poll_min, NULL);
+
+static umode_t input_poller_attrs_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct input_dev *input = to_input_dev(dev);
+
+ return input->poller ? attr->mode : 0;
+}
+
+static struct attribute *input_poller_attrs[] = {
+ &dev_attr_poll.attr,
+ &dev_attr_max.attr,
+ &dev_attr_min.attr,
+ NULL
+};
+
+struct attribute_group input_poller_attribute_group = {
+ .is_visible = input_poller_attrs_visible,
+ .attrs = input_poller_attrs,
+};
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 7f3c5fcb9ed6..6b87c07d5981 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -24,6 +24,7 @@
#include <linux/mutex.h>
#include <linux/rcupdate.h>
#include "input-compat.h"
+#include "input-poller.h"
MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
MODULE_DESCRIPTION("Input core");
@@ -603,20 +604,31 @@ int input_open_device(struct input_handle *handle)
handle->open++;
- if (!dev->users++ && dev->open)
- retval = dev->open(dev);
+ if (dev->users++) {
+ /*
+ * Device is already opened, so we can exit immediately and
+ * report success.
+ */
+ goto out;
+ }
- if (retval) {
- dev->users--;
- if (!--handle->open) {
+ if (dev->open) {
+ retval = dev->open(dev);
+ if (retval) {
+ dev->users--;
+ handle->open--;
/*
* Make sure we are not delivering any more events
* through this handle
*/
synchronize_rcu();
+ goto out;
}
}
+ if (dev->poller)
+ input_dev_poller_start(dev->poller);
+
out:
mutex_unlock(&dev->mutex);
return retval;
@@ -655,8 +667,13 @@ void input_close_device(struct input_handle *handle)
__input_release_device(handle);
- if (!--dev->users && dev->close)
- dev->close(dev);
+ if (!--dev->users) {
+ if (dev->poller)
+ input_dev_poller_stop(dev->poller);
+
+ if (dev->close)
+ dev->close(dev);
+ }
if (!--handle->open) {
/*
@@ -1502,6 +1519,7 @@ static const struct attribute_group *input_dev_attr_groups[] = {
&input_dev_attr_group,
&input_dev_id_attr_group,
&input_dev_caps_attr_group,
+ &input_poller_attribute_group,
NULL
};
@@ -1511,6 +1529,7 @@ static void input_dev_release(struct device *device)
input_ff_destroy(dev);
input_mt_destroy_slots(dev);
+ kfree(dev->poller);
kfree(dev->absinfo);
kfree(dev->vals);
kfree(dev);
@@ -2135,6 +2154,9 @@ int input_register_device(struct input_dev *dev)
if (!dev->setkeycode)
dev->setkeycode = input_default_setkeycode;
+ if (dev->poller)
+ input_dev_poller_finalize(dev->poller);
+
error = device_add(&dev->dev);
if (error)
goto err_free_vals;
diff --git a/include/linux/input.h b/include/linux/input.h
index 510e78558c10..956f32be87cc 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -21,6 +21,8 @@
#include <linux/timer.h>
#include <linux/mod_devicetable.h>
+struct input_dev_poller;
+
/**
* struct input_value - input value representation
* @type: type of value (EV_KEY, EV_ABS, etc)
@@ -147,6 +149,8 @@ struct input_dev {
struct ff_device *ff;
+ struct input_dev_poller *poller;
+
unsigned int repeat_key;
struct timer_list timer;
@@ -361,6 +365,12 @@ void input_unregister_device(struct input_dev *);
void input_reset_device(struct input_dev *);
+int input_setup_polling(struct input_dev *dev,
+ void (*poll_fn)(struct input_dev *dev));
+void input_set_poll_interval(struct input_dev *dev, unsigned int interval);
+void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval);
+void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval);
+
int __must_check input_register_handler(struct input_handler *);
void input_unregister_handler(struct input_handler *);
--
Dmitry
^ permalink raw reply related
* Re: [PATCH v15 06/13] irqchip: Add irq-ingenic-tcu driver
From: Marc Zyngier @ 2019-07-25 8:21 UTC (permalink / raw)
To: Paul Cercueil, Ralf Baechle, Paul Burton, James Hogan,
Jonathan Corbet, Lee Jones, Arnd Bergmann, Daniel Lezcano,
Thomas Gleixner, Michael Turquette, Stephen Boyd, Jason Cooper,
Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, linux-doc, linux-mips, linux-clk, od,
Mathieu Malaterre, Artur Rojek
In-Reply-To: <20190724171615.20774-7-paul@crapouillou.net>
On 24/07/2019 18:16, Paul Cercueil wrote:
> This driver handles the interrupt controller built in the Timer/Counter
> Unit (TCU) of the JZ47xx SoCs from Ingenic.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Tested-by: Mathieu Malaterre <malat@debian.org>
> Tested-by: Artur Rojek <contact@artur-rojek.eu>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Marc Zyngier <maz@kernel.org>
Given the various dependencies, I assume the series will get routed via
the MIPS tree.
Thanks,
M.
--
Jazz is not dead, it just smells funny...
^ 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