* [PATCH v2 3/8] PM / Domains: Allow domain power states to be read from DT
From: Ulf Hansson @ 2016-10-10 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475879821-8035-4-git-send-email-lina.iyer@linaro.org>
On 8 October 2016 at 00:36, Lina Iyer <lina.iyer@linaro.org> wrote:
> This patch allows domains to define idle states in the DT. SoC's can
> define domain idle states in DT using the "domain-idle-states" property
> of the domain provider. Add API to read the idle states from DT that can
> be set in the genpd object.
>
> This patch is based on the original patch by Marc Titinger.
>
> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
> drivers/base/power/domain.c | 95 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pm_domain.h | 8 ++++
> 2 files changed, 103 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 4e87170..4208b67 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1917,6 +1917,101 @@ out:
> return ret ? -EPROBE_DEFER : 0;
> }
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
> +
> +static const struct of_device_id idle_state_match[] = {
> + { .compatible = "arm,idle-state", },
> + { }
> +};
> +
> +static int genpd_parse_state(struct genpd_power_state *genpd_state,
> + struct device_node *state_node)
> +{
> + int err;
> + u32 residency;
> + u32 entry_latency, exit_latency;
> + const struct of_device_id *match_id;
> +
> + match_id = of_match_node(idle_state_match, state_node);
> + if (!match_id)
> + return -EINVAL;
> +
> + err = of_property_read_u32(state_node, "entry-latency-us",
> + &entry_latency);
> + if (err) {
> + pr_debug(" * %s missing entry-latency-us property\n",
> + state_node->full_name);
> + return -EINVAL;
> + }
> +
> + err = of_property_read_u32(state_node, "exit-latency-us",
> + &exit_latency);
> + if (err) {
> + pr_debug(" * %s missing exit-latency-us property\n",
> + state_node->full_name);
> + return -EINVAL;
> + }
> +
> + err = of_property_read_u32(state_node, "min-residency-us", &residency);
> + if (!err)
> + genpd_state->residency_ns = 1000 * residency;
> +
> + genpd_state->power_on_latency_ns = 1000 * exit_latency;
> + genpd_state->power_off_latency_ns = 1000 * entry_latency;
> +
> + return 0;
> +}
> +
> +/**
> + * of_genpd_parse_idle_states: Return array of idle states for the genpd.
> + *
> + * @dn: The genpd device node
> + * @states: The pointer to which the state array will be saved.
> + * @n: The count of elements in the array returned from this function.
> + *
> + * Returns the device states parsed from the OF node. The memory for the states
> + * is allocated by this function and is the responsibility of the caller to
> + * free the memory after use.
> + */
> +int of_genpd_parse_idle_states(struct device_node *dn,
> + struct genpd_power_state **states, int *n)
Instead of taking **states as a parameter, let's instead return it as
a pointer for the allocated struct. In case of failures, let's return
ERR_PTR().
> +{
> + struct genpd_power_state *st;
> + struct device_node *np;
> + int i = 0;
> + int err, ret;
> + int count;
> + struct of_phandle_iterator it;
> +
> + count = of_count_phandle_with_args(dn, "domain-idle-states", NULL);
If count is zero or an error, we should return an error code (ERR_PTR()). Right?
> +
> + st = kcalloc(count, sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> +
> + /* Loop over the phandles until all the requested entry is found */
> + of_for_each_phandle(&it, err, dn, "domain-idle-states", NULL, 0) {
> + np = of_node_get(it.node);
I don't think you need to increment the usage count for the device
node as that is already managed by of_for_each_phandle().
It's only in the error case below, when it's needed.
> + ret = genpd_parse_state(&st[i++], np);
> + if (ret) {
> + pr_err
> + ("Parsing idle state node %s failed with err %d\n",
> + np->full_name, ret);
> + of_node_put(np);
> + goto fail;
The goto seems unnecessary. Why not deal with all error handling here
and return the error code?
> + }
> + of_node_put(np);
According the comment above, you should be able to remove this.
> + }
> +
> + *n = count;
> + *states = st;
> +
> + return 0;
> +fail:
> + kfree(st);
> + return ret;
> +}
> +EXPORT_SYMBOL(of_genpd_parse_idle_states);
Please use EXPORT_SYMBOL_GPL() instead.
> +
> #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
>
>
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index f4492eb..b489496 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -205,6 +205,8 @@ extern int of_genpd_add_device(struct of_phandle_args *args,
> extern int of_genpd_add_subdomain(struct of_phandle_args *parent,
> struct of_phandle_args *new_subdomain);
> extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
> +extern int of_genpd_parse_idle_states(struct device_node *dn,
> + struct genpd_power_state **states, int *n);
>
> int genpd_dev_pm_attach(struct device *dev);
> #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
> @@ -234,6 +236,12 @@ static inline int of_genpd_add_subdomain(struct of_phandle_args *parent,
> return -ENODEV;
> }
>
> +static inline int of_genpd_parse_idle_states(struct device_node *dn,
> + struct genpd_power_state **states, int *n)
> +{
> + return -ENODEV;
> +}
> +
> static inline int genpd_dev_pm_attach(struct device *dev)
> {
> return -ENODEV;
> --
> 2.7.4
>
Kind regards
Uffe
^ permalink raw reply
* [PATCH v2 0/2] clk: imx: fix AV PLL rate setting
From: Emil Lundmark @ 2016-10-10 10:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I did not use a cover letter in v1, so here is a summary instead.
I discovered a problem when trying to set the rate of the audio PLL
(pll4_post_div) on an i.MX6Q. The rate I wanted to set was 196.608 MHz, but
the actual rate I got was 192.000570 MHz. This patch series fixes this
issue and also improves the precision of the audio/video PLL.
There are no code changes in v2, only clarifications in the commit
messages.
Changes since v1:
- Use correct subsystem name in commit summary.
- Use 'Fixes:' tag to indicate bug fix.
- Revise argument for choosing the denominator register value after
comments from Lothar Wa?mann.
v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460350.html
Emil Lundmark (2):
clk: imx: fix integer overflow in AV PLL round rate
clk: imx: improve precision of AV PLL to 1 Hz
drivers/clk/imx/clk-pllv3.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
--
2.7.4
^ permalink raw reply
* [PATCH v2 1/2] clk: imx: fix integer overflow in AV PLL round rate
From: Emil Lundmark @ 2016-10-10 10:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1476092427.git.emil@limesaudio.com>
Since 'parent_rate * mfn' may overflow 32 bits, the result should be
stored using 64 bits.
Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
Signed-off-by: Emil Lundmark <emil@limesaudio.com>
---
drivers/clk/imx/clk-pllv3.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index 19f9b622981a..bc7f163ea13c 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -247,7 +247,11 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
do_div(temp64, parent_rate);
mfn = temp64;
- return parent_rate * div + parent_rate * mfn / mfd;
+ temp64 = (u64)parent_rate;
+ temp64 *= mfn;
+ do_div(temp64, mfd);
+
+ return parent_rate * div + (u32)temp64;
}
static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/2] clk: imx: improve precision of AV PLL to 1 Hz
From: Emil Lundmark @ 2016-10-10 10:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1476092427.git.emil@limesaudio.com>
The audio and video PLLs are designed to have a precision of 1 Hz if some
conditions are met. The current implementation only allows a precision that
depends on the rate of the parent clock. E.g., if the parent clock is 24
MHz, the precision will be 24 Hz; or more generally the precision will be
p / 10^6 Hz
where p is the parent clock rate. This comes down to how the register
values for the PLL's fractional loop divider are chosen.
The clock rate calculation for the PLL is
PLL output frequency = Fref * (DIV_SELECT + NUM / DENOM)
or with a shorter notation
r = p * (d + a / b)
In addition to all variables being integers, we also have the following
conditions:
27 <= d <= 54
-2^29 <= a <= 2^29-1
0 < b <= 2^30-1
|a| < b
Here, d, a and b are register values for the fractional loop divider. We
want to chose d, a and b such that f(p, r) = p, i.e. f is our round_rate
function. Currently, d and b are chosen as
d = r / p
b = 10^6
hence we get the poor precision. And a is defined in terms of r, d, p and
b:
a = (r - d * p) * b / p
I propose that if p <= 2^30-1 (i.e., the max value for b), we chose b as
b = p
We can do this since
|a| < b
|(r - d * p) * b / p| < b
|r - d * p| < p
Which have two solutions, one of them is when p < 0, so we can skip that
one. The other is when p > 0 and
p * (d - 1) < r < p * (d + 1)
Substitute d = r / p:
(r - p) < r < (r + p) <=> p > 0
So, as long as p > 0, we can chose b = p. This is a good choice for b since
a = (r - d * p) * b / p
= r - d * p
r = p * (d + a / b)
= p * d + p * a / b
= p * d + a
and if d = r / p:
a = r - d * p
= r - r / p * p
= 0
r = p * d + a
= p * d + 0
= p * r / p
= r
I reckon this is the intention by the design of the clock rate formula.
Signed-off-by: Emil Lundmark <emil@limesaudio.com>
---
drivers/clk/imx/clk-pllv3.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index bc7f163ea13c..5ff55ff5fd81 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -234,6 +234,7 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long max_rate = parent_rate * 54;
u32 div;
u32 mfn, mfd = 1000000;
+ u32 max_mfd = 0x3FFFFFFF;
u64 temp64;
if (rate > max_rate)
@@ -241,6 +242,9 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
else if (rate < min_rate)
rate = min_rate;
+ if (parent_rate <= max_mfd)
+ mfd = parent_rate;
+
div = rate / parent_rate;
temp64 = (u64) (rate - div * parent_rate);
temp64 *= mfd;
@@ -262,11 +266,15 @@ static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long max_rate = parent_rate * 54;
u32 val, div;
u32 mfn, mfd = 1000000;
+ u32 max_mfd = 0x3FFFFFFF;
u64 temp64;
if (rate < min_rate || rate > max_rate)
return -EINVAL;
+ if (parent_rate <= max_mfd)
+ mfd = parent_rate;
+
div = rate / parent_rate;
temp64 = (u64) (rate - div * parent_rate);
temp64 *= mfd;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 4/8] PM / Domains: Save the fwnode in genpd_power_state
From: Ulf Hansson @ 2016-10-10 10:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475879821-8035-5-git-send-email-lina.iyer@linaro.org>
On 8 October 2016 at 00:36, Lina Iyer <lina.iyer@linaro.org> wrote:
> Save the fwnode for the genpd state in the state node. PM Domain clients
> may use the fwnode to read in the platform specific domain state
> properties and associate them with the state.
>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> drivers/base/power/domain.c | 1 +
> include/linux/pm_domain.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 4208b67..e0f31fe 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1957,6 +1957,7 @@ static int genpd_parse_state(struct genpd_power_state *genpd_state,
>
> genpd_state->power_on_latency_ns = 1000 * exit_latency;
> genpd_state->power_off_latency_ns = 1000 * entry_latency;
> + genpd_state->fwnode = &state_node->fwnode;
>
> return 0;
> }
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index b489496..6a89881 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -39,6 +39,7 @@ struct genpd_power_state {
> s64 power_off_latency_ns;
> s64 power_on_latency_ns;
> s64 residency_ns;
> + struct fwnode_handle *fwnode;
> };
>
> struct generic_pm_domain {
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH 05/12] ASoC: sun4i-codec: Add support for A31 playback through headphone output
From: Maxime Ripard @ 2016-10-10 10:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v66b_zG=xK_JVBnwY=SPtvJ+c-fZjuugAHUybzZdkkcScg@mail.gmail.com>
On Tue, Oct 04, 2016 at 12:26:08PM +0800, Chen-Yu Tsai wrote:
> >> +struct sun4i_codec_regs {
> >> + u32 adc_fifoc;
> >> + u32 adc_fifos;
> >> + u32 adc_rxdata;
> >> +};
> >> +
> >> struct sun4i_codec {
> >> struct device *dev;
> >> struct regmap *regmap;
> >> struct clk *clk_apb;
> >> struct clk *clk_module;
> >> struct gpio_desc *gpio_pa;
> >> + const struct sun4i_codec_regs *regs;
> >
> > You're reimplementing reg_field here.
>
> Are you suggesting we do reg_fields for each register?
> Or all the bit fields separately. The latter would add
> quite a few pointers.
only the one that change, so judging from your structure, only the ADC
fifo control, status and data registers.
> >> +static const struct of_device_id sun4i_codec_of_match[] = {
> >> + {
> >> + .compatible = "allwinner,sun4i-a10-codec",
> >> + .data = &sun4i_codec_quirks,
> >> + },
> >> + {
> >> + .compatible = "allwinner,sun6i-a31-codec",
> >> + .data = &sun6i_a31_codec_quirks,
> >> + },
> >> + {
> >> + .compatible = "allwinner,sun7i-a20-codec",
> >> + .data = &sun7i_codec_quirks,
> >> + },
> >> + {}
> >> +};
> >> +MODULE_DEVICE_TABLE(of, sun4i_codec_of_match);
> >> +
> >
> > I don't really like moving blocks of code over and over again,
> > especially in the middle of an unrelated patch.
>
> It's not completely unrelated. I want different create_card
> functions for the different SoCs, and that has to be part of the
> quirks, so the quirks and the of_match list have to be moved
> below them. I suppose I could leave the regmap parts in place,
> but keeping them together is nicer.
>
> If I split out the addition of the .create_card field and
> code movement into a separate patch, would that be OK?
Yep, that would work.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161010/0827457c/attachment.sig>
^ permalink raw reply
* [PATCH] Reorganize STM32 clocks in order to prepare them for PLLI2S and PLLSAI
From: Daniel Thompson @ 2016-10-10 10:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFvLkMQjDZysgoT3FfkVP8UQVkqghW9=CZU-OEmGUAdMR7CO2A@mail.gmail.com>
On 10/10/16 10:56, Rados?aw Pietrzyk wrote:
> Hi,
> all plls have the same clock parent which is after a main divider.
> Currently the divider and multiplier are connected together within vco
> clock and therefore there is no chance to reuse the divider and clearly
> state where the conncetion "really" is. We can arrange all of them
> separately but than the divider will be hidden for all of them separately.
Quoting my last mail "I can see the value of naming the "/M"
pre-division separately". In other words I agree with the idea of the patch.
To more explicitly state my review comments...
> From: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
Please add a explanation of the problem and solution in the patch
description.
> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
> ---
> drivers/clk/clk-stm32f4.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
> index 02d6810..1fd3eac 100644
> --- a/drivers/clk/clk-stm32f4.c
> +++ b/drivers/clk/clk-stm32f4.c
> @@ -245,9 +245,10 @@ static void stm32f4_rcc_register_pll(const char
*hse_clk, const char *hsi_clk)
> const char *pllsrc = pllcfgr & BIT(22) ? hse_clk : hsi_clk;
> unsigned long pllq = (pllcfgr >> 24) & 0xf;
>
> - clk_register_fixed_factor(NULL, "vco", pllsrc, 0, plln, pllm);
> - clk_register_fixed_factor(NULL, "pll", "vco", 0, 1, pllp);
> - clk_register_fixed_factor(NULL, "pll48", "vco", 0, 1, pllq);
> + clk_register_fixed_factor(NULL, "vco-div", pllsrc, 0, 1, pllm);
This strikes me as a bad name for a clock that is shared by all three
PLLs (the vco being an internal component of the PLL) however since the
clock is not named in the datasheet we are forced to invent a name [I
suspect that's why I gave up trying to name it when I wrote the driver
originally ;-) ].
Perhaps "pllin-prediv"?
> + clk_register_fixed_factor(NULL, "vco-mul", "vco-div", 0, plln, 1);
Why rename this clock? Multiplying is a what the vco (and its control
circuits) is *for*. Tagging it "-mul" is meaningless.
Daniel.
^ permalink raw reply
* [PATCH] MAINTAINERS: Add ARM64-specific ACPI maintainers entry
From: Will Deacon @ 2016-10-10 10:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161005112540.22189-1-lorenzo.pieralisi@arm.com>
On Wed, Oct 05, 2016 at 12:25:40PM +0100, Lorenzo Pieralisi wrote:
> The ARM64 architecture defines ARM64 specific ACPI bindings to
> configure and set-up arch specific components. To simplify
> code reviews/updates and streamline the maintainership structure
> supporting the arch specific code, a new arm64 directory was created in
> /drivers/acpi, to contain ACPI code that is specific to ARM64
> architecture.
>
> Add the ARM64-specific ACPI maintainers entry in MAINTAINERS for
> the newly created subdirectory and respective code content.
>
> Lorenzo Pieralisi will be in charge of submitting and managing
> the pull requests on behalf of all maintainers listed.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Link: http://lkml.kernel.org/r/1603704.EGiVTcCxLR at vostro.rjw.lan
> ---
> MAINTAINERS | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f593300..2a70dd9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -316,6 +316,14 @@ W: https://01.org/linux-acpi
> S: Supported
> F: drivers/acpi/fan.c
>
> +ACPI FOR ARM64 (ACPI/arm64)
> +M: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> +M: Hanjun Guo <hanjun.guo@linaro.org>
> +M: Sudeep Holla <sudeep.holla@arm.com>
> +L: linux-acpi at vger.kernel.org
> +S: Maintained
> +F: drivers/acpi/arm64
Thanks for doing this:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply
* [PATCH v6 0/6] ARM: dts: imx6q: Add Engicam i.CoreM6 dts
From: Jagan Teki @ 2016-10-10 10:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD6G_RRw68FVxG5rum41OeSrb9wDD0SkKeDgwjRLSJ2BuVtqfw@mail.gmail.com>
Hi Shawn,
On Fri, Oct 7, 2016 at 7:51 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Tue, Sep 27, 2016 at 12:53 AM, Jagan Teki <jteki@openedev.com> wrote:
>> This is series add dts support for Engicam I.Core M6 qdl modules on
>> top of */shawnguo/linux.git for-next.
>>
>> Jagan Teki (6):
>> of: Add vendor prefix for Engicam s.r.l company
This patch applied already.
>> ARM: dts: imx6q: Add Engicam i.CoreM6 Quad/Dual initial support
>> ARM: dts: imx6q: Add Engicam i.CoreM6 DualLite/Solo initial support
>> ARM: dts: imx6qdl-icore: Add usbhost support
>> ARM: dts: imx6qdl-icore: Add usbotg support
>> ARM: dts: imx6qdl-icore: Add FEC support
Please take these five patches, I was trying to rebase but no resent
*/git/shawnguo/linux.git tree.
thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply
* [RESEND PATCH v6, 3/5] usb: xhci-mtk: make IPPC register optional
From: Matthias Brugger @ 2016-10-10 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474437277-27201-4-git-send-email-chunfeng.yun@mediatek.com>
On 09/21/2016 07:54 AM, Chunfeng Yun wrote:
> Make IPPC register optional to support host side of dual-role mode,
> due to it is moved into common glue layer for simplification.
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> drivers/usb/host/xhci-mtk.c | 36 +++++++++++++++++++++++++++++-------
> 1 file changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 79959f1..4bf99b9 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -94,6 +94,9 @@ static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
> int ret;
> int i;
>
> + if (ippc == NULL)
> + return 0;
> +
> /* power on host ip */
> value = readl(&ippc->ip_pw_ctr1);
> value &= ~CTRL1_IP_HOST_PDN;
> @@ -139,6 +142,9 @@ static int xhci_mtk_host_disable(struct xhci_hcd_mtk *mtk)
> int ret;
> int i;
>
> + if (ippc == NULL)
> + return 0;
> +
> /* power down all u3 ports */
> for (i = 0; i < mtk->num_u3_ports; i++) {
> value = readl(&ippc->u3_ctrl_p[i]);
> @@ -173,6 +179,9 @@ static int xhci_mtk_ssusb_config(struct xhci_hcd_mtk *mtk)
> struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
> u32 value;
>
> + if (ippc == NULL)
> + return 0;
> +
I would prefer to add a flag/bool in xhci_hcd_mtk to signal the absence
of the ippc. Or at least use a macro which checks the presence before
calling any of this three functions.
Regards,
Matthias
> /* reset whole ip */
> value = readl(&ippc->ip_pw_ctr0);
> value |= CTRL0_IP_SW_RST;
> @@ -475,6 +484,7 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> /* called during probe() after chip reset completes */
> static int xhci_mtk_setup(struct usb_hcd *hcd)
> {
> + struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> struct xhci_hcd_mtk *mtk = hcd_to_mtk(hcd);
> int ret;
>
> @@ -482,12 +492,21 @@ static int xhci_mtk_setup(struct usb_hcd *hcd)
> ret = xhci_mtk_ssusb_config(mtk);
> if (ret)
> return ret;
> + }
> +
> + ret = xhci_gen_setup(hcd, xhci_mtk_quirks);
> + if (ret)
> + return ret;
> +
> + if (usb_hcd_is_primary_hcd(hcd)) {
> + mtk->num_u3_ports = xhci->num_usb3_ports;
> + mtk->num_u2_ports = xhci->num_usb2_ports;
> ret = xhci_mtk_sch_init(mtk);
> if (ret)
> return ret;
> }
>
> - return xhci_gen_setup(hcd, xhci_mtk_quirks);
> + return ret;
> }
>
> static int xhci_mtk_probe(struct platform_device *pdev)
> @@ -586,7 +605,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> mtk->hcd = platform_get_drvdata(pdev);
> platform_set_drvdata(pdev, mtk);
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mac");
> hcd->regs = devm_ioremap_resource(dev, res);
> if (IS_ERR(hcd->regs)) {
> ret = PTR_ERR(hcd->regs);
> @@ -595,11 +614,14 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> hcd->rsrc_start = res->start;
> hcd->rsrc_len = resource_size(res);
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - mtk->ippc_regs = devm_ioremap_resource(dev, res);
> - if (IS_ERR(mtk->ippc_regs)) {
> - ret = PTR_ERR(mtk->ippc_regs);
> - goto put_usb2_hcd;
> + mtk->ippc_regs = NULL;
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ippc");
> + if (res) { /* ippc register is optional */
> + mtk->ippc_regs = devm_ioremap_resource(dev, res);
> + if (IS_ERR(mtk->ippc_regs)) {
> + ret = PTR_ERR(mtk->ippc_regs);
> + goto put_usb2_hcd;
> + }
> }
>
> for (phy_num = 0; phy_num < mtk->num_phys; phy_num++) {
>
^ permalink raw reply
* [PATCH v2 7/8] PM / Domains: Support IRQ safe PM domains
From: Ulf Hansson @ 2016-10-10 11:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475879821-8035-8-git-send-email-lina.iyer@linaro.org>
On 8 October 2016 at 00:37, Lina Iyer <lina.iyer@linaro.org> wrote:
> Generic Power Domains currently support turning on/off only in process
> context. This prevents the usage of PM domains for domains that could be
> powered on/off in a context where IRQs are disabled. Many such domains
> exist today and do not get powered off, when the IRQ safe devices in
> that domain are powered off, because of this limitation.
>
> However, not all domains can operate in IRQ safe contexts. Genpd
> therefore, has to support both cases where the domain may or may not
> operate in IRQ safe contexts. Configuring genpd to use an appropriate
> lock for that domain, would allow domains that have IRQ safe devices to
> runtime suspend and resume, in atomic context.
>
> To achieve domain specific locking, set the domain's ->flag to
> GENPD_FLAG_IRQ_SAFE while defining the domain. This indicates that genpd
> should use a spinlock instead of a mutex for locking the domain. Locking
> is abstracted through genpd_lock() and genpd_unlock() functions that use
> the flag to determine the appropriate lock to be used for that domain.
>
> Domains that have lower latency to suspend and resume and can operate
> with IRQs disabled may now be able to save power, when the component
> devices and sub-domains are idle at runtime.
>
> The restriction this imposes on the domain hierarchy is that non-IRQ
> safe domains may not have IRQ-safe subdomains, but IRQ safe domains may
> have IRQ safe and non-IRQ safe subdomains and devices.
>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> drivers/base/power/domain.c | 111 ++++++++++++++++++++++++++++++++++++++++----
> include/linux/pm_domain.h | 10 +++-
> 2 files changed, 110 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index d0ae559..87a016a 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -74,11 +74,70 @@ static const struct genpd_lock_ops genpd_mtx_ops = {
> .unlock = genpd_unlock_mtx,
> };
>
> +static void genpd_lock_spin(struct generic_pm_domain *genpd)
> + __acquires(&genpd->slock)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&genpd->slock, flags);
> + genpd->lock_flags = flags;
> +}
> +
> +static void genpd_lock_nested_spin(struct generic_pm_domain *genpd,
> + int depth)
> + __acquires(&genpd->slock)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave_nested(&genpd->slock, flags, depth);
> + genpd->lock_flags = flags;
> +}
> +
> +static int genpd_lock_interruptible_spin(struct generic_pm_domain *genpd)
> + __acquires(&genpd->slock)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&genpd->slock, flags);
> + genpd->lock_flags = flags;
> + return 0;
> +}
> +
> +static void genpd_unlock_spin(struct generic_pm_domain *genpd)
> + __releases(&genpd->slock)
> +{
> + spin_unlock_irqrestore(&genpd->slock, genpd->lock_flags);
> +}
> +
> +static const struct genpd_lock_ops genpd_spin_ops = {
> + .lock = genpd_lock_spin,
> + .lock_nested = genpd_lock_nested_spin,
> + .lock_interruptible = genpd_lock_interruptible_spin,
> + .unlock = genpd_unlock_spin,
> +};
> +
> #define genpd_lock(p) p->lock_ops->lock(p)
> #define genpd_lock_nested(p, d) p->lock_ops->lock_nested(p, d)
> #define genpd_lock_interruptible(p) p->lock_ops->lock_interruptible(p)
> #define genpd_unlock(p) p->lock_ops->unlock(p)
>
> +#define genpd_is_irq_safe(genpd) (genpd->flags & GENPD_FLAG_IRQ_SAFE)
> +
> +static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
> + struct generic_pm_domain *genpd)
> +{
> + bool ret;
> +
> + ret = pm_runtime_is_irq_safe(dev) && !genpd_is_irq_safe(genpd);
> +
> + /* Warn once for each IRQ safe dev in no sleep domain */
> + if (ret)
> + dev_warn_once(dev, "PM domain %s will not be powered off\n",
> + genpd->name);
> +
> + return ret;
> +}
> +
> /*
> * Get the generic PM domain for a particular struct device.
> * This validates the struct device pointer, the PM domain pointer,
> @@ -343,7 +402,12 @@ static int genpd_poweroff(struct generic_pm_domain *genpd, bool is_async)
> if (stat > PM_QOS_FLAGS_NONE)
> return -EBUSY;
>
> - if (!pm_runtime_suspended(pdd->dev) || pdd->dev->power.irq_safe)
> + /*
> + * Do not allow PM domain to be powered off, when an IRQ safe
> + * device is part of a non-IRQ safe domain.
> + */
> + if (!pm_runtime_suspended(pdd->dev) ||
> + irq_safe_dev_in_no_sleep_domain(pdd->dev, genpd))
> not_suspended++;
> }
>
> @@ -506,10 +570,10 @@ static int genpd_runtime_suspend(struct device *dev)
> }
>
> /*
> - * If power.irq_safe is set, this routine will be run with interrupts
> - * off, so it can't use mutexes.
> + * If power.irq_safe is set, this routine may be run with
> + * IRQs disabled, so suspend only if the PM domain also is irq_safe.
> */
> - if (dev->power.irq_safe)
> + if (irq_safe_dev_in_no_sleep_domain(dev, genpd))
> return 0;
>
> genpd_lock(genpd);
> @@ -543,8 +607,11 @@ static int genpd_runtime_resume(struct device *dev)
> if (IS_ERR(genpd))
> return -EINVAL;
>
> - /* If power.irq_safe, the PM domain is never powered off. */
> - if (dev->power.irq_safe) {
> + /*
> + * As we don't power off a non IRQ safe domain, which holds
> + * an IRQ safe device, we don't need to restore power to it.
> + */
> + if (irq_safe_dev_in_no_sleep_domain(dev, genpd)) {
> timed = false;
> goto out;
> }
> @@ -586,7 +653,8 @@ static int genpd_runtime_resume(struct device *dev)
> err_stop:
> genpd_stop_dev(genpd, dev);
> err_poweroff:
> - if (!dev->power.irq_safe) {
> + if (!pm_runtime_is_irq_safe(dev) ||
> + (pm_runtime_is_irq_safe(dev) && genpd_is_irq_safe(genpd))) {
> genpd_lock(genpd);
> genpd_poweroff(genpd, 0);
> genpd_unlock(genpd);
> @@ -1223,6 +1291,17 @@ static int genpd_add_subdomain(struct generic_pm_domain *genpd,
> || genpd == subdomain)
> return -EINVAL;
>
> + /*
> + * If the domain can be powered on/off in an IRQ safe
> + * context, ensure that the subdomain can also be
> + * powered on/off in that context.
> + */
> + if (!genpd_is_irq_safe(genpd) && genpd_is_irq_safe(subdomain)) {
> + WARN("Parent %s of subdomain %s must be IRQ safe\n",
> + genpd->name, subdomain->name);
> + return -EINVAL;
> + }
> +
> link = kzalloc(sizeof(*link), GFP_KERNEL);
> if (!link)
> return -ENOMEM;
> @@ -1337,6 +1416,17 @@ static int genpd_set_default_power_state(struct generic_pm_domain *genpd)
> return 0;
> }
>
> +static void genpd_lock_init(struct generic_pm_domain *genpd)
> +{
> + if (genpd->flags & GENPD_FLAG_IRQ_SAFE) {
> + spin_lock_init(&genpd->slock);
> + genpd->lock_ops = &genpd_spin_ops;
> + } else {
> + mutex_init(&genpd->mlock);
> + genpd->lock_ops = &genpd_mtx_ops;
> + }
> +}
> +
> /**
> * pm_genpd_init - Initialize a generic I/O PM domain object.
> * @genpd: PM domain object to initialize.
> @@ -1356,8 +1446,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
> INIT_LIST_HEAD(&genpd->master_links);
> INIT_LIST_HEAD(&genpd->slave_links);
> INIT_LIST_HEAD(&genpd->dev_list);
> - mutex_init(&genpd->mlock);
> - genpd->lock_ops = &genpd_mtx_ops;
> + genpd_lock_init(genpd);
> genpd->gov = gov;
> INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
> atomic_set(&genpd->sd_count, 0);
> @@ -2133,7 +2222,9 @@ static int pm_genpd_summary_one(struct seq_file *s,
> }
>
> list_for_each_entry(pm_data, &genpd->dev_list, list_node) {
> - kobj_path = kobject_get_path(&pm_data->dev->kobj, GFP_KERNEL);
> + kobj_path = kobject_get_path(&pm_data->dev->kobj,
> + genpd_is_irq_safe(genpd) ?
> + GFP_ATOMIC : GFP_KERNEL);
> if (kobj_path == NULL)
> continue;
>
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 811b968..81ece61 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -15,9 +15,11 @@
> #include <linux/err.h>
> #include <linux/of.h>
> #include <linux/notifier.h>
> +#include <linux/spinlock.h>
>
> /* Defines used for the flags field in the struct generic_pm_domain */
> #define GENPD_FLAG_PM_CLK (1U << 0) /* PM domain uses PM clk */
> +#define GENPD_FLAG_IRQ_SAFE (1U << 1) /* PM domain operates in atomic */
>
> enum gpd_status {
> GPD_STATE_ACTIVE = 0, /* PM domain is active */
> @@ -76,7 +78,13 @@ struct generic_pm_domain {
> unsigned int state_idx; /* state that genpd will go to when off */
> void *free; /* Free the state that was allocated for default */
> const struct genpd_lock_ops *lock_ops;
> - struct mutex mlock;
> + union {
> + struct mutex mlock;
> + struct {
> + spinlock_t slock;
> + unsigned long lock_flags;
> + };
> + };
>
> };
>
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v2 8/8] PM / doc: Update device documentation for devices in IRQ safe PM domains
From: Ulf Hansson @ 2016-10-10 11:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475879821-8035-9-git-send-email-lina.iyer@linaro.org>
On 8 October 2016 at 00:37, Lina Iyer <lina.iyer@linaro.org> wrote:
> Update documentation to reflect the changes made to support IRQ safe PM
> domains.
>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> Documentation/power/devices.txt | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
> index 8ba6625..0401b53 100644
> --- a/Documentation/power/devices.txt
> +++ b/Documentation/power/devices.txt
> @@ -607,7 +607,14 @@ individually. Instead, a set of devices sharing a power resource can be put
> into a low-power state together at the same time by turning off the shared
> power resource. Of course, they also need to be put into the full-power state
> together, by turning the shared power resource on. A set of devices with this
> -property is often referred to as a power domain.
> +property is often referred to as a power domain. A power domain may also be
> +nested inside another power domain.
> +
> +Devices and PM domains may be defined as IRQ-safe, if they can be powered
> +on/off even when the IRQs are disabled. An IRQ-safe device in a domain will
> +disallow power management on the domain, unless the domain is also defined as
> +IRQ-safe. The restriction this framework imposes on the parent domain of an
> +IRQ-safe domain is that it must also be defined as IRQ-safe.
>
> Support for power domains is provided through the pm_domain field of struct
> device. This field is a pointer to an object of type struct dev_pm_domain,
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH] tty: serial: fsl_lpuart: Fix Tx DMA edge case
From: Bhuvanchandra DV @ 2016-10-10 11:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475791984-17705-1-git-send-email-aaron.brice@datasoft.com>
On 10/07/16 03:43, Aaron Brice wrote:
> In the case where head == 0 on the circular buffer, there should be one
> DMA buffer, not two. The second zero-length buffer would break the
> lpuart driver, transfer would never complete.
Tested-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
>
> Signed-off-by: Aaron Brice <aaron.brice@datasoft.com>
> ---
> drivers/tty/serial/fsl_lpuart.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index de9d510..76103f2 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -328,7 +328,7 @@ static void lpuart_dma_tx(struct lpuart_port *sport)
>
> sport->dma_tx_bytes = uart_circ_chars_pending(xmit);
>
> - if (xmit->tail < xmit->head) {
> + if (xmit->tail < xmit->head || xmit->head == 0) {
> sport->dma_tx_nents = 1;
> sg_init_one(sgl, xmit->buf + xmit->tail, sport->dma_tx_bytes);
> } else {
> @@ -359,7 +359,6 @@ static void lpuart_dma_tx(struct lpuart_port *sport)
> sport->dma_tx_in_progress = true;
> sport->dma_tx_cookie = dmaengine_submit(sport->dma_tx_desc);
> dma_async_issue_pending(sport->dma_tx_chan);
> -
> }
>
> static void lpuart_dma_tx_complete(void *arg)
^ permalink raw reply
* [PATCH v2 1/5] clk: add support for runtime pm
From: Marek Szyprowski @ 2016-10-10 11:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFqtGTJ38K5xm+-xVZcjAmLuo21613GHwUkQKKJXvYtsLQ@mail.gmail.com>
Hi Ulf,
Thanks for your comments!
On 2016-10-07 12:07, Ulf Hansson wrote:
> On 19 September 2016 at 12:55, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> Registers for some clocks might be located in the SOC area, which are under the
>> power domain. To enable access to those registers respective domain has to be
>> turned on. Additionally, registers for such clocks will usually loose its
>> contents when power domain is turned off, so additional saving and restoring of
>> them might be needed in the clock controller driver.
>>
>> This patch adds basic infrastructure in the clocks core to allow implementing
>> driver for such clocks under power domains. Clock provider can supply a
>> struct device pointer, which is the used by clock core for tracking and managing
>> clock's controller runtime pm state. Each clk_prepare() operation
>> will first call pm_runtime_get_sync() on the supplied device, while
>> clk_unprepare() will do pm_runtime_put() at the end.
>>
>> Additional calls to pm_runtime_get/put functions are required to ensure that any
>> register access (like calculating/changing clock rates and unpreparing/disabling
>> unused clocks on boot) will be done with clock controller in runtime resumend
>> state.
>>
>> When one wants to register clock controller, which make use of this feature, he
>> has to:
>> 1. Provide a struct device to the core when registering the provider and set
>> CLK_RUNTIME_PM flags for its clocks.
>> 2. It needs to enable runtime PM for that device.
>> 3. It needs to make sure the runtime PM status of the controller device reflects
>> the HW state.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> drivers/clk/clk.c | 107 +++++++++++++++++++++++++++++++++++++++----
>> include/linux/clk-provider.h | 1 +
>> 2 files changed, 98 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 820a939fb6bb..096a199b8e46 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -21,6 +21,7 @@
>> #include <linux/of.h>
>> #include <linux/device.h>
>> #include <linux/init.h>
>> +#include <linux/pm_runtime.h>
>> #include <linux/sched.h>
>> #include <linux/clkdev.h>
>>
>> @@ -46,6 +47,7 @@ struct clk_core {
>> const struct clk_ops *ops;
>> struct clk_hw *hw;
>> struct module *owner;
>> + struct device *dev;
>> struct clk_core *parent;
>> const char **parent_names;
>> struct clk_core **parents;
>> @@ -87,6 +89,26 @@ struct clk {
>> struct hlist_node clks_node;
>> };
>>
>> +/*** runtime pm ***/
>> +static int clk_pm_runtime_get(struct clk_core *core)
>> +{
>> + int ret = 0;
>> +
>> + if (!core->dev)
>> + return 0;
>> +
>> + ret = pm_runtime_get_sync(core->dev);
>> + return ret < 0 ? ret : 0;
>> +}
>> +
>> +static void clk_pm_runtime_put(struct clk_core *core)
>> +{
>> + if (!core->dev)
>> + return;
>> +
>> + pm_runtime_put(core->dev);
>> +}
>> +
>> /*** locking ***/
>> static void clk_prepare_lock(void)
>> {
>> @@ -150,6 +172,8 @@ static void clk_enable_unlock(unsigned long flags)
>>
>> static bool clk_core_is_prepared(struct clk_core *core)
>> {
>> + bool status;
>> +
>> /*
>> * .is_prepared is optional for clocks that can prepare
>> * fall back to software usage counter if it is missing
>> @@ -157,11 +181,17 @@ static bool clk_core_is_prepared(struct clk_core *core)
>> if (!core->ops->is_prepared)
>> return core->prepare_count;
>>
>> - return core->ops->is_prepared(core->hw);
>> + clk_pm_runtime_get(core);
> I guess you should assign status to the return code, and check it.
Okay. I assume that in case of any failure from runtime pm, the function
should return false?
>
>> + status = core->ops->is_prepared(core->hw);
>> + clk_pm_runtime_put(core);
>> +
>> + return status;
>> }
>>
>> static bool clk_core_is_enabled(struct clk_core *core)
>> {
>> + bool status;
>> +
>> /*
>> * .is_enabled is only mandatory for clocks that gate
>> * fall back to software usage counter if .is_enabled is missing
>> @@ -169,7 +199,29 @@ static bool clk_core_is_enabled(struct clk_core *core)
>> if (!core->ops->is_enabled)
>> return core->enable_count;
>>
>> - return core->ops->is_enabled(core->hw);
>> + /*
>> + * Check if runtime pm is enabled before calling .is_enabled callback,
>> + * if not assume that clock is disabled, because we might be called
>> + * from atomic context, from which pm_runtime_get() is not allowed.
>> + * This function is called mainly from clk_disable_unused_subtree,
>> + * which ensures proper runtime pm activation of controller before
>> + * taking enable spinlock, but the below check is needed if one tries
>> + * to call it from other place.
>> + */
>> + if (core->dev) {
>> + pm_runtime_get_noresume(core->dev);
>> + if (pm_runtime_suspended(core->dev)) {
> I think it's wrong to use pm_runtime_suspended().
>
> What you should be checking, is whether the device is RPM_ACTIVE or if
> runtime PM isn't enabled for device.
>
> In other words, you should use pm_runtime_active() to find out whether
> it's okay to invoke the ->is_enabled() ops or not.
>
> Accordingly, I think the upper comment you added then needs to be a
> rephrased a bit to reflect this.
Okay, I will change it to use pm_runtime_active(). It looks that mentally
I still assume that runtime pm will be disabled in system sleep transitions
phase, so the only check that provided some useful information about
previous runtime pm state was pm_runtime_suspended().
>
>> + status = false;
>> + goto done;
>> + }
>> + }
>> +
>> + status = core->ops->is_enabled(core->hw);
>> +done:
>> + if (core->dev)
>> + pm_runtime_put(core->dev);
>> +
>> + return status;
>> }
>>
>> /*** helper functions ***/
>> @@ -489,6 +541,8 @@ static void clk_core_unprepare(struct clk_core *core)
>> if (core->ops->unprepare)
>> core->ops->unprepare(core->hw);
>>
>> + clk_pm_runtime_put(core);
>> +
>> trace_clk_unprepare_complete(core);
>> clk_core_unprepare(core->parent);
>> }
>> @@ -530,10 +584,14 @@ static int clk_core_prepare(struct clk_core *core)
>> return 0;
>>
>> if (core->prepare_count == 0) {
>> - ret = clk_core_prepare(core->parent);
>> + ret = clk_pm_runtime_get(core);
>> if (ret)
>> return ret;
>>
>> + ret = clk_core_prepare(core->parent);
>> + if (ret)
>> + goto runtime_put;
>> +
>> trace_clk_prepare(core);
>>
>> if (core->ops->prepare)
>> @@ -541,15 +599,18 @@ static int clk_core_prepare(struct clk_core *core)
>>
>> trace_clk_prepare_complete(core);
>>
>> - if (ret) {
>> - clk_core_unprepare(core->parent);
>> - return ret;
>> - }
>> + if (ret)
>> + goto unprepare;
>> }
>>
>> core->prepare_count++;
>>
>> return 0;
>> +unprepare:
>> + clk_core_unprepare(core->parent);
>> +runtime_put:
>> + clk_pm_runtime_put(core);
>> + return ret;
>> }
>>
>> static int clk_core_prepare_lock(struct clk_core *core)
>> @@ -745,6 +806,9 @@ static void clk_unprepare_unused_subtree(struct clk_core *core)
>> if (core->flags & CLK_IGNORE_UNUSED)
>> return;
>>
>> + if (clk_pm_runtime_get(core) != 0)
> You may simplify this:
> if (clk_pm_runtime_get(core))
>
>> + return;
>> +
>> if (clk_core_is_prepared(core)) {
>> trace_clk_unprepare(core);
>> if (core->ops->unprepare_unused)
>> @@ -753,6 +817,8 @@ static void clk_unprepare_unused_subtree(struct clk_core *core)
>> core->ops->unprepare(core->hw);
>> trace_clk_unprepare_complete(core);
>> }
>> +
>> + clk_pm_runtime_put(core);
>> }
>>
>> static void clk_disable_unused_subtree(struct clk_core *core)
>> @@ -768,6 +834,9 @@ static void clk_disable_unused_subtree(struct clk_core *core)
>> if (core->flags & CLK_OPS_PARENT_ENABLE)
>> clk_core_prepare_enable(core->parent);
>>
>> + if (clk_pm_runtime_get(core) != 0)
> Is there any reason to why you haven't moved this further down in this
> function, like just before calling clk_core_is_enabled()?
Yes, clk_enable_lock() takes a spinlock, so I cannot call pm_runtime_get
after it.
>
> You may also simplify this:
> if (clk_pm_runtime_get(core))
>
>> + return;
>> +
> You need to restore the call made to clk_core_prepare_enable()
> earlier, so please update the error handling to cope with this.
>
>> flags = clk_enable_lock();
>>
>> if (core->enable_count)
>> @@ -794,6 +863,8 @@ unlock_out:
>> clk_enable_unlock(flags);
>> if (core->flags & CLK_OPS_PARENT_ENABLE)
>> clk_core_disable_unprepare(core->parent);
>> +
>> + clk_pm_runtime_put(core);
>> }
>>
>> static bool clk_ignore_unused;
>> @@ -1563,6 +1634,7 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
>> {
>> struct clk_core *top, *fail_clk;
>> unsigned long rate = req_rate;
>> + int ret = 0;
>>
>> if (!core)
>> return 0;
>> @@ -1579,21 +1651,28 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
>> if (!top)
>> return -EINVAL;
>>
>> + ret = clk_pm_runtime_get(core);
>> + if (ret)
>> + return ret;
>> +
>> /* notify that we are about to change rates */
>> fail_clk = clk_propagate_rate_change(top, PRE_RATE_CHANGE);
>> if (fail_clk) {
>> pr_debug("%s: failed to set %s rate\n", __func__,
>> fail_clk->name);
>> clk_propagate_rate_change(top, ABORT_RATE_CHANGE);
>> - return -EBUSY;
>> + ret = -EBUSY;
>> + goto err;
>> }
>>
>> /* change the rates */
>> clk_change_rate(top);
>>
>> core->req_rate = req_rate;
>> +err:
>> + clk_pm_runtime_put(core);
>>
>> - return 0;
>> + return ret;
>> }
>>
>> /**
>> @@ -1824,12 +1903,16 @@ static int clk_core_set_parent(struct clk_core *core, struct clk_core *parent)
>> p_rate = parent->rate;
>> }
>>
>> + ret = clk_pm_runtime_get(core);
>> + if (ret)
>> + goto out;
>> +
>> /* propagate PRE_RATE_CHANGE notifications */
>> ret = __clk_speculate_rates(core, p_rate);
>>
>> /* abort if a driver objects */
>> if (ret & NOTIFY_STOP_MASK)
>> - goto out;
>> + goto runtime_put;
>>
>> /* do the re-parent */
>> ret = __clk_set_parent(core, parent, p_index);
>> @@ -1842,6 +1925,8 @@ static int clk_core_set_parent(struct clk_core *core, struct clk_core *parent)
>> __clk_recalc_accuracies(core);
>> }
>>
>> +runtime_put:
>> + clk_pm_runtime_put(core);
>> out:
>> clk_prepare_unlock();
>>
>> @@ -2546,6 +2631,8 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
>> goto fail_name;
>> }
>> core->ops = hw->init->ops;
>> + if (dev && (hw->init->flags & CLK_RUNTIME_PM))
>> + core->dev = dev;
> I guess you need this to play safe, although I am really wondering if
> we should try without.
>
> Not that many clocks are currently being registered with a valid
> struct device pointer. For the other cases why not try to use runtime
> PM as per default?
I've that tried initially, but it causes failure for all the clock
controllers, which don't enable runtime pm. One of such case is max77686
PMIC, which provides 3 clocks. Maybe a negative flag (CLK_NO_RUNTIME_PM)
will be a better solution, so by default the runtime pm calls will be
enabled for every driver providing struct device?
> Moreover we anyway rely on the clock provider to enable runtime PM for
> the clock device, and when that isn't the case the runtime PM
> deployment in the core should still be safe, right!?
I don't get the above comment. Do you want to check if runtime pm has
been enabled during clock registration?
>> if (dev && dev->driver)
>> core->owner = dev->driver->owner;
>> core->hw = hw;
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index a39c0c530778..8a131eb71fdf 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -35,6 +35,7 @@
>> #define CLK_IS_CRITICAL BIT(11) /* do not gate, ever */
>> /* parents need enable during gate/ungate, set rate and re-parent */
>> #define CLK_OPS_PARENT_ENABLE BIT(12)
>> +#define CLK_RUNTIME_PM BIT(13)
>>
>> struct clk;
>> struct clk_hw;
>> --
>> 1.9.1
>>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* [PATCH v1 1/2] ARM: dts: add rockchip PX3 Evaluation board
From: Andy Yan @ 2016-10-10 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <738050204.FfkOZgkGGl@phil>
Hi Heiko:
On 2016?10?10? 17:54, Heiko Stuebner wrote:
> Hi Andreas, Andy,
>
> Am Dienstag, 13. September 2016, 14:14:01 CEST schrieb Andreas F?rber:
>> Hi Andy,
>>
>> This patch didn't make it to linux-rockchip list somehow...
>> Not sure why I'm CC'ed, I don't have access to such a board to check, so
>> just a couple formal nitpicks:
>>
>> Am 10.09.2016 um 19:44 schrieb Andy Yan:
>>> PX3 EVB is designed by Rockchip for automotive field,
>>> which intergrated with CVBS(TP2825)/MIPI DSI/LVDS/HDMI
>> "integrated"
>> but the grammar is somewhat incorrect with "which" referring to field -
>> I assume you meant "with integrated CVBS..."?
>>
>>> video input/output interface, WIFI/BT/GPS(on a module
>> Also please always leave a space before an opening parenthesis in
>> English text. Similarly above, spaces around "/" would help recognize
>> that MIPI DSI belongs together rather than being two lists.
>>
>> If nothing else applies below then maybe Heiko can edit it for you?
> I've fixed the remarks in the commit description.
>
>
>>> named S500 which based on MT6620), Gsensor BMA250E and
>>> light&proximity sensor STK3410.
>>>
>>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>>>
>>> ---
>>>
>>> Changes in v1:
>>> - board rename
>>> - add vendor prefix for i2c interfaced sensors
>>> - use stdout-path to set the default console
>>>
>>> Documentation/devicetree/bindings/arm/rockchip.txt | 4 +
>>> arch/arm/boot/dts/Makefile | 1 +
>>> arch/arm/boot/dts/rk3188-px3-evb.dts | 337
>>> +++++++++++++++++++++ 3 files changed, 342 insertions(+)
>>> create mode 100644 arch/arm/boot/dts/rk3188-px3-evb.dts
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/rockchip.txt
>>> b/Documentation/devicetree/bindings/arm/rockchip.txt index
>>> 6668645..6da3881 100644
>>> --- a/Documentation/devicetree/bindings/arm/rockchip.txt
>>> +++ b/Documentation/devicetree/bindings/arm/rockchip.txt
>>> @@ -21,6 +21,10 @@ Rockchip platforms device tree bindings
>>>
>>> Required root node properties:
>>> - compatible = "radxa,rock", "rockchip,rk3188";
>>>
>>> +- Rockchip PX3 Evaluation board:
>>> + Required root node properties:
>>> + - compatible = "rockchip,px3-evb", "rockchip,px3",
>>> "rockchip,rk3188";
>> How compatible is PX3 with RK3188? It is a separate SoC product:
>>
>> http://www.rock-chips.com/a/en/products/rkpower/2015/1125/730.html
>>
>> Wondering whether or not to drop the third compatible string.
> As discussed in IRC (and with arm-soc maintainers), I intend to keep the
> rk3188 part, as the chip really is just a (hardened?) variant of the consumer
> rk3188, but shares the same internals with the original.
>
>>> +
>>>
>>> - Radxa Rock2 Square board:
>>> Required root node properties:
>>> - compatible = "radxa,rock2-square", "rockchip,rk3288";
>>>
>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>> index faacd52..88d27a2 100644
>>> --- a/arch/arm/boot/dts/Makefile
>>> +++ b/arch/arm/boot/dts/Makefile
>>> @@ -620,6 +620,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
>>>
>>> rk3066a-marsboard.dtb \
>>> rk3066a-rayeager.dtb \
>>> rk3188-radxarock.dtb \
>>>
>>> + rk3188-px3-evb.dtb \
>> Affects file naming as well: px3-evb.dtb?
> see above
>
>>> rk3228-evb.dtb \
>>> rk3229-evb.dtb \
>>> rk3288-evb-act8846.dtb \
>>>
>>> diff --git a/arch/arm/boot/dts/rk3188-px3-evb.dts
>>> b/arch/arm/boot/dts/rk3188-px3-evb.dts new file mode 100644
>>> index 0000000..f6bc738
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/rk3188-px3-evb.dts
>>> @@ -0,0 +1,337 @@
>>> +/*
>>> + * Copyright (c) 2016 Andy Yan <andy.yan@rock-chips.com>
>>> + *
>>> + * This file is dual-licensed: you can use it either under the terms
>>> + * of the GPL or the X11 license, at your option. Note that this dual
>>> + * licensing only applies to this file, and not this project as a
>>> + * whole.
>>> + *
>>> + * a) This file is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License as
>>> + * published by the Free Software Foundation; either version 2 of the
>>> + * License, or (at your option) any later version.
>>> + *
>>> + * This file is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * Or, alternatively,
>>> + *
>>> + * b) Permission is hereby granted, free of charge, to any person
>>> + * obtaining a copy of this software and associated documentation
>>> + * files (the "Software"), to deal in the Software without
>>> + * restriction, including without limitation the rights to use,
>>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>>> + * sell copies of the Software, and to permit persons to whom the
>>> + * Software is furnished to do so, subject to the following
>>> + * conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be
>>> + * included in all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include <dt-bindings/input/input.h>
>>> +#include "rk3188.dtsi"
>> I'm surprised there is no [rk3188-]px3.dtsi here! Surely some automotive
>> vendor may design their own board with it and should have at least the
>> two trailing compatible strings pre-set.
> see above. I don't think there is a need to pollute the directory with
> (nearly) empty dtsi files, especially as the compatible will get overwritten by
> a board compatible anyway.
>
>>> +
>>> +/ {
>>> + model = "Rockchip PX3-EVB";
>>> + compatible = "rockchip,px3-evb", "rockchip,px3", "rockchip,rk3188";
>>> +
>>> + chosen {
>>> + stdout-path = "serial2:115200n8";
>>> + };
>>> +
>>> + memory {
>>> + device_type = "memory";
>>> + reg = <0x60000000 0x80000000>;
>>> + };
>>> +
>>> + gpio-keys {
>>> + compatible = "gpio-keys";
>>> + autorepeat;
>>> +
>>> + power {
>>> + gpios = <&gpio0 4 GPIO_ACTIVE_LOW>;
>>> + linux,code = <KEY_POWER>;
>>> + label = "GPIO Key Power";
>>> + linux,input-type = <1>;
>>> + wakeup-source;
>>> + debounce-interval = <100>;
>>> + };
>>> + };
>>> +
>>> + vcc_sys: vsys-regulator {
>>> + compatible = "regulator-fixed";
>>> + regulator-name = "vsys";
>>> + regulator-min-microvolt = <5000000>;
>>> + regulator-max-microvolt = <5000000>;
>>> + regulator-boot-on;
>>> + };
>>> +};
>>> +
>>> +&cpu0 {
>>> + cpu0-supply = <&vdd_cpu>;
>>> +};
>>> +
>>> +&i2c0 {
>>> + status = "okay";
>>> +
>>> + /* Accelerometer */
>> Space after tab intentional?
>>
>>> + bma250 at 18 {
>>> + compatible = "bosch,bma250";
>>> + reg = <0x18>;
>>> + interrupt-parent = <&gpio0>;
>>> + interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
>>> + };
>>> +
>>> + stk3410 at 48 {
>>> + compatible = "sensortek,STK3310";
>>> + reg = <0x48>;
>>> + interrupt-parent = <&gpio1>;
>>> + interrupts = <5 IRQ_TYPE_LEVEL_LOW>;
>>> + };
>> Generally it is undesired to repeat the compatible name as node name -
>> did you compare other .dts files? (e.g., accelerometer at 18 would be
>> self-documenting) If this is a copy from an existing .dts then please
>> ignore this comment.
> due to the compatible ambiguity, I had dropped the stk3410 node anyway.
> I've also checked how the bma250 gets specified and there are both variants in
> use (accelerometer@ and bmc250@). So to set a good example for the future,
> I've changed the node name to accelerometer at 18 and dropped the now self
> explanatory comment obove it.
>
>
>>> +};
>>> +
>>> +&i2c1 {
>>> + status = "okay";
>>> + clock-frequency = <400000>;
>> Insert white line?
> done
>
>
> Heiko
>
>
I have no problem with these above, Thanks for all you have done.
^ permalink raw reply
* [PATCH 0/6] crypto: arm64 - big endian fixes
From: Ard Biesheuvel @ 2016-10-10 11:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476034945-9186-1-git-send-email-ard.biesheuvel@linaro.org>
On 9 October 2016 at 18:42, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> As it turns out, none of the accelerated crypto routines under arch/arm64/crypto
> currently work, or have ever worked correctly when built for big endian. So this
> series fixes all of them.
>
> Each of these patches carries a fixes tag, and could be backported to stable.
> However, for patches #1 and #5, the fixes tag denotes the oldest commit that the
> fix is compatible with, not the patch that introduced the algorithm. This is due
> to the fact that the key schedules are incompatible between generic AES and the
> arm64 Crypto Extensions implementation (but only when building for big endian)
> This is not a problem in practice, but it does mean that the AES-CCM and AES in
> EBC/CBC/CTR/XTS mode implementations before v3.19 require a different fix, i.e.,
> one that is compatible with the generic AES key schedule generation code (which
> it currently no longer uses)
>
> In any case, please apply with cc to stable.
>
Herbert,
I have an additional fix to add to this series, and a couple for
32-bit ARM as well. They escaped my attention due to this code (in
algboss.c:250)
/* This piece of crap needs to disappear into per-type test hooks. */
if (!((type ^ CRYPTO_ALG_TYPE_BLKCIPHER) &
CRYPTO_ALG_TYPE_BLKCIPHER_MASK) && !(type & CRYPTO_ALG_GENIV) &&
((alg->cra_flags & CRYPTO_ALG_TYPE_MASK) ==
CRYPTO_ALG_TYPE_BLKCIPHER ? alg->cra_blkcipher.ivsize :
alg->cra_ablkcipher.ivsize))
type |= CRYPTO_ALG_TESTED;
This causes cbc(aes), ctr(aes) and xts(aes) to remain untested, unless
I add CRYPTO_ALG_GENIV to their cra_flags. Is this expected behavior?
What would be your recommended way to ensure these algos are covered
by the boottime tests?
Thanks,
Ard.
^ permalink raw reply
* [PATCH] Reorganize STM32 clocks in order to prepare them for PLLI2S and PLLSAI
From: Alexandre Torgue @ 2016-10-10 11:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b6fd21b7-75a6-c38d-a9f1-fbf07132a1d3@linaro.org>
Hi Radoslaw,
I add Gabriel in the discussion. Gabriel is updating PLL management for
STM32F429.
Regards
Alex
On 10/10/2016 12:31 PM, Daniel Thompson wrote:
> On 10/10/16 10:56, Rados?aw Pietrzyk wrote:
>> Hi,
>> all plls have the same clock parent which is after a main divider.
>> Currently the divider and multiplier are connected together within vco
>> clock and therefore there is no chance to reuse the divider and clearly
>> state where the conncetion "really" is. We can arrange all of them
>> separately but than the divider will be hidden for all of them
>> separately.
>
> Quoting my last mail "I can see the value of naming the "/M"
> pre-division separately". In other words I agree with the idea of the
> patch.
>
> To more explicitly state my review comments...
>
>> From: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>
> Please add a explanation of the problem and solution in the patch
> description.
>
>
>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>> ---
>> drivers/clk/clk-stm32f4.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
>> index 02d6810..1fd3eac 100644
>> --- a/drivers/clk/clk-stm32f4.c
>> +++ b/drivers/clk/clk-stm32f4.c
>> @@ -245,9 +245,10 @@ static void stm32f4_rcc_register_pll(const char
> *hse_clk, const char *hsi_clk)
>> const char *pllsrc = pllcfgr & BIT(22) ? hse_clk : hsi_clk;
>> unsigned long pllq = (pllcfgr >> 24) & 0xf;
>>
>> - clk_register_fixed_factor(NULL, "vco", pllsrc, 0, plln, pllm);
>> - clk_register_fixed_factor(NULL, "pll", "vco", 0, 1, pllp);
>> - clk_register_fixed_factor(NULL, "pll48", "vco", 0, 1, pllq);
>> + clk_register_fixed_factor(NULL, "vco-div", pllsrc, 0, 1, pllm);
>
> This strikes me as a bad name for a clock that is shared by all three
> PLLs (the vco being an internal component of the PLL) however since the
> clock is not named in the datasheet we are forced to invent a name [I
> suspect that's why I gave up trying to name it when I wrote the driver
> originally ;-) ].
>
> Perhaps "pllin-prediv"?
>
>
>> + clk_register_fixed_factor(NULL, "vco-mul", "vco-div", 0, plln, 1);
>
> Why rename this clock? Multiplying is a what the vco (and its control
> circuits) is *for*. Tagging it "-mul" is meaningless.
>
>
> Daniel.
^ permalink raw reply
* [PATCH 1/1] mfd: Remove unused variable using Coccinelle
From: Linus Walleij @ 2016-10-10 11:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476063419-13866-1-git-send-email-mayhs11saini@gmail.com>
On Mon, Oct 10, 2016 at 3:36 AM, Shyam Saini <mayhs11saini@gmail.com> wrote:
> The variable err is initialized but never used otherwise.
>
> The semantic patch that makes this change is as follows:
>
> // <smpl>
> @@
> type T;
> identifier i;
> constant C;
> @@
>
> (
> extern T i;
> |
> - T i;
> <+... when != i
> - i = C;
> ...+>
> )
> // </smpl>
>
> Signed-off-by: Shyam Saini <mayhs11saini@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] Reorganize STM32 clocks in order to prepare them for PLLI2S and PLLSAI
From: Gabriel Fernandez @ 2016-10-10 12:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1880a125-b5ec-b64a-5c9b-90d4e9c0af86@st.com>
Hi Rados?aw,
Yes i m nearly ready to push a patch-set to manage LCD-TFT clock.
In my patch-set i introduced PLLI2S and PLLSAI in generic way, and offer
the possibility to change the vco frequency (in order to cover all
frequencies for any LCD).
And then, the vco is no longer a fixed factor.
This patch is just a fix or do you planned to upstream PLLI2S and PLLSAI ?
If you are ok I can send my patch-set ?
Best Regards
Gabriel
On 10/10/2016 01:32 PM, Alexandre Torgue wrote:
> Hi Radoslaw,
>
> I add Gabriel in the discussion. Gabriel is updating PLL management
> for STM32F429.
>
> Regards
> Alex
>
> On 10/10/2016 12:31 PM, Daniel Thompson wrote:
>> On 10/10/16 10:56, Rados?aw Pietrzyk wrote:
>>> Hi,
>>> all plls have the same clock parent which is after a main divider.
>>> Currently the divider and multiplier are connected together within vco
>>> clock and therefore there is no chance to reuse the divider and clearly
>>> state where the conncetion "really" is. We can arrange all of them
>>> separately but than the divider will be hidden for all of them
>>> separately.
>>
>> Quoting my last mail "I can see the value of naming the "/M"
>> pre-division separately". In other words I agree with the idea of the
>> patch.
>>
>> To more explicitly state my review comments...
>>
>>> From: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>>
>> Please add a explanation of the problem and solution in the patch
>> description.
>>
>>
>>> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
>>> ---
>>> drivers/clk/clk-stm32f4.c | 7 ++++---
>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
>>> index 02d6810..1fd3eac 100644
>>> --- a/drivers/clk/clk-stm32f4.c
>>> +++ b/drivers/clk/clk-stm32f4.c
>>> @@ -245,9 +245,10 @@ static void stm32f4_rcc_register_pll(const char
>> *hse_clk, const char *hsi_clk)
>>> const char *pllsrc = pllcfgr & BIT(22) ? hse_clk : hsi_clk;
>>> unsigned long pllq = (pllcfgr >> 24) & 0xf;
>>>
>>> - clk_register_fixed_factor(NULL, "vco", pllsrc, 0, plln, pllm);
>>> - clk_register_fixed_factor(NULL, "pll", "vco", 0, 1, pllp);
>>> - clk_register_fixed_factor(NULL, "pll48", "vco", 0, 1, pllq);
>>> + clk_register_fixed_factor(NULL, "vco-div", pllsrc, 0, 1, pllm);
>>
>> This strikes me as a bad name for a clock that is shared by all three
>> PLLs (the vco being an internal component of the PLL) however since the
>> clock is not named in the datasheet we are forced to invent a name [I
>> suspect that's why I gave up trying to name it when I wrote the driver
>> originally ;-) ].
>>
>> Perhaps "pllin-prediv"?
>>
>>
>>> + clk_register_fixed_factor(NULL, "vco-mul", "vco-div", 0, plln, 1);
>>
>> Why rename this clock? Multiplying is a what the vco (and its control
>> circuits) is *for*. Tagging it "-mul" is meaningless.
>>
>>
>> Daniel.
^ permalink raw reply
* [PATCH v1 2/2] arm64: dts: rockchip: Add PX5 Evaluation board
From: Heiko Stuebner @ 2016-10-10 12:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ef2bb52c-26f9-49a5-fe18-538effcd30b0@suse.de>
Hi Andreas, Andy,
Am Dienstag, 13. September 2016, 14:23:35 CEST schrieb Andreas F?rber:
> Hi,
>
> Am 10.09.2016 um 19:47 schrieb Andy Yan:
> > PX5 EVB is designed by Rockchip for automotive field,
> > which intergrated with CVBS(TP2825)/MIPI DSI/CSI/LVDS
> > HDMI video input/output interface, audio codec ES8396,
> > WIFI/BT(on RTL8723BS), Gsensor BMA250E and light&proximity
> > sensor STK3410.
> >
> > Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>
> Most comments from PX3 1/2 apply here, too.
ammended te commit
> > ---
> >
> > Changes in v1:
> > - board rename
> > - add vendor prefix for i2c interfaced devices
> >
> > Documentation/devicetree/bindings/arm/rockchip.txt | 4 +
> > arch/arm64/boot/dts/rockchip/Makefile | 1 +
> > arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts | 304
> > +++++++++++++++++++++ 3 files changed, 309 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts
> >
> > diff --git a/Documentation/devicetree/bindings/arm/rockchip.txt
> > b/Documentation/devicetree/bindings/arm/rockchip.txt index
> > 6da3881..b6f92d6 100644
> > --- a/Documentation/devicetree/bindings/arm/rockchip.txt
> > +++ b/Documentation/devicetree/bindings/arm/rockchip.txt
> > @@ -107,6 +107,10 @@ Rockchip platforms device tree bindings
> >
> > Required root node properties:
> > - compatible = "rockchip,r88", "rockchip,rk3368";
> >
> > +- Rockchip PX5 Evaluation board:
> > + Required root node properties:
> > + - compatible = "rockchip,px5-evb", "rockchip,px5",
> > "rockchip,rk3368"; +
> >
> > - Rockchip RK3228 Evaluation board:
> > Required root node properties:
> > - compatible = "rockchip,rk3228-evb", "rockchip,rk3228";
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile
> > b/arch/arm64/boot/dts/rockchip/Makefile index 7037a16..86c74b2 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -1,6 +1,7 @@
> >
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-evb-act8846.dtb
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-geekbox.dtb
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-r88.dtb
> >
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-px5-evb.dtb
>
> There is no PX5 listed on your English website (yet), but given that you
> have your own rockchip/ folder for arm64 I would encourage you to rename
> to px5-evb.dtb independent of any internal heritage or compatibility the
> SoC has. Compare apq/ipq/msm in qcom.
I prefer to keep it like it is, see comment in px3 patch.
>
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-evb.dtb
> >
> > always := $(dtb-y)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts
> > b/arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts new file mode 100644
> > index 0000000..be0e915
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3368-px5-evb.dts
> > @@ -0,0 +1,304 @@
> > +/*
> > + * Copyright (c) 2016 Fuzhou Rockchip Electronics Co., Ltd
> > + *
> > + * This file is dual-licensed: you can use it either under the terms
> > + * of the GPL or the X11 license, at your option. Note that this dual
> > + * licensing only applies to this file, and not this project as a
> > + * whole.
> > + *
> > + * a) This file is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of the
> > + * License, or (at your option) any later version.
> > + *
> > + * This file is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * Or, alternatively,
> > + *
> > + * b) Permission is hereby granted, free of charge, to any person
> > + * obtaining a copy of this software and associated documentation
> > + * files (the "Software"), to deal in the Software without
> > + * restriction, including without limitation the rights to use,
> > + * copy, modify, merge, publish, distribute, sublicense, and/or
> > + * sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following
> > + * conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be
> > + * included in all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> > + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> > + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> > + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > + * OTHER DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +/dts-v1/;
> > +#include "rk3368.dtsi"
>
> Similarly suggest a px5.dtsi here.
>
> > +#include <dt-bindings/input/input.h>
> > +
> > +/ {
> > + model = "Rockchip PX5 EVB";
> > + compatible = "rockchip,px5-evb", "rockchip,px5", "rockchip,rk3368";
> > +
> > + chosen {
> > + stdout-path = "serial4:115200n8";
> > + };
> > +
> > + memory at 0 {
> > + device_type = "memory";
> > + reg = <0x0 0x0 0x0 0x80000000>;
> > + };
> > +
> > + keys: gpio-keys {
> > + compatible = "gpio-keys";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pwr_key>;
> > +
> > + power {
> > + gpios = <&gpio0 2 GPIO_ACTIVE_LOW>;
> > + label = "GPIO Power";
> > + linux,code = <KEY_POWER>;
> > + wakeup-source;
> > + };
> > + };
> > +
> > + vcc_sys: vcc-sys-regulator {
> > + compatible = "regulator-fixed";
> > + regulator-name = "vcc_sys";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + regulator-always-on;
> > + regulator-boot-on;
> > + };
> > +};
> > +
> > +&emmc {
> > + status = "okay";
> > + bus-width = <8>;
> > + cap-mmc-highspeed;
> > + clock-frequency = <150000000>;
> > + disable-wp;
> > + keep-power-in-suspend;
> > + non-removable;
> > + num-slots = <1>;
> > + vmmc-supply = <&vcc_io>;
> > + vqmmc-supply = <&vcc18_flash>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&emmc_clk>, <&emmc_cmd>, <&emmc_bus8>;
> > +};
> > +
> > +&i2c0 {
> > + status = "okay";
> > +
> > + rk808: pmic at 1b {
> > + compatible = "rockchip,rk808";
> > + reg = <0x1b>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pmic_int>, <&pmic_sleep>;
> > + interrupt-parent = <&gpio0>;
> > + interrupts = <5 IRQ_TYPE_LEVEL_LOW>;
> > + rockchip,system-power-controller;
> > + vcc1-supply = <&vcc_sys>;
> > + vcc2-supply = <&vcc_sys>;
> > + vcc3-supply = <&vcc_sys>;
> > + vcc4-supply = <&vcc_sys>;
> > + vcc6-supply = <&vcc_sys>;
> > + vcc7-supply = <&vcc_sys>;
> > + vcc8-supply = <&vcc_io>;
> > + vcc9-supply = <&vcc_sys>;
> > + vcc10-supply = <&vcc_sys>;
> > + vcc11-supply = <&vcc_sys>;
> > + vcc12-supply = <&vcc_io>;
> > + clock-output-names = "xin32k", "rk808-clkout2";
> > + #clock-cells = <1>;
> > +
> > + regulators {
> > + vdd_cpu: DCDC_REG1 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <1500000>;
> > + regulator-name = "vdd_cpu";
> > + };
> > +
> > + vdd_log: DCDC_REG2 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <1500000>;
> > + regulator-name = "vdd_log";
> > + };
> > +
> > + vcc_ddr: DCDC_REG3 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-name = "vcc_ddr";
> > + };
> > +
> > + vcc_io: DCDC_REG4 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-name = "vcc_io";
> > + };
> > +
> > + vcc18_flash: LDO_REG1 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <1800000>;
> > + regulator-name = "vcc18_flash";
> > + };
> > +
> > + vcca_33: LDO_REG2 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-name = "vcca_33";
> > + };
> > +
> > + vdd_10: LDO_REG3 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <1000000>;
> > + regulator-max-microvolt = <1000000>;
> > + regulator-name = "vdd_10";
> > + };
> > +
> > + avdd_33: LDO_REG4 {
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-name = "avdd_33";
> > + };
> > +
> > + vccio_sd: LDO_REG5 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-name = "vccio_sd";
> > + };
> > +
> > + vdd10_lcd: LDO_REG6 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <1000000>;
> > + regulator-max-microvolt = <1000000>;
> > + regulator-name = "vdd10_lcd";
> > + };
> > +
> > + vcc_18: LDO_REG7 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <1800000>;
> > + regulator-name = "vcc_18";
> > + };
> > +
> > + vcc18_lcd: LDO_REG8 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <1800000>;
> > + regulator-name = "vcc18_lcd";
> > + };
> > +
> > + vcc_sd: SWITCH_REG1 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-name = "vcc_sd";
> > + };
> > +
> > + vcc33_lcd: SWITCH_REG2 {
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-name = "vcc33_lcd";
> > + };
> > + };
> > + };
> > +};
> > +
> > +&i2c1 {
> > + status = "okay";
> > +
> > + /* Accelerometer */
> > + bma250 at 18 {
changed to accelerometer at 18 similar to px3
> > + compatible = "bosch,bma250";
> > + reg = <0x18>;
> > + interrupt-parent = <&gpio2>;
> > + interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> > + };
> > +
> > + stk3410 at 48 {
> > + compatible = "sensortek,STK3310";
> > + reg = <0x48>;
> > + interrupt-parent = <&gpio2>;
> > + interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
> > + };
> > +};
> > +
> > +&i2c2 {
> > + status = "okay";
> > +
> > + gsl1680: touchscreen at 40 {
> > + compatible = "silead,gsl1680";
> > + reg = <0x40>;
> > + interrupt-parent = <&gpio3>;
> > + interrupts = <28 IRQ_TYPE_EDGE_FALLING>;
> > + power-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>;
> > + touchscreen-size-x = <800>;
> > + touchscreen-size-y = <1280>;
> > + silead,max-fingers = <5>;
> > + };
> > +};
> > +
> > +&pinctrl {
> > +
>
> Drop white line?
seems I dropped that already.
> > + keys {
> > + pwr_key: pwr-key {
> > + rockchip,pins = <0 2 RK_FUNC_GPIO &pcfg_pull_none>;
> > + };
> > + };
> > +
> > + pmic {
> > + pmic_sleep: pmic-sleep {
> > + rockchip,pins = <0 0 RK_FUNC_2 &pcfg_pull_none>;
> > + };
> > +
> > + pmic_int: pmic-int {
> > + rockchip,pins = <0 5 RK_FUNC_GPIO &pcfg_pull_up>;
> > + };
> > + };
> > +};
> > +
> > +&tsadc {
> > + status = "okay";
> > + rockchip,hw-tshut-mode = <0>; /* CRU */
> > + rockchip,hw-tshut-polarity = <1>; /* high */
> > +};
> > +
> > +&uart4 {
> > + status = "okay";
> > +};
> > +
> > +&usb_host0_ehci {
> > + status = "okay";
> > +};
> > +
> > +&usb_otg {
> > + status = "okay";
> > +};
> > +
> > +&wdt {
> > + status = "okay";
> > +};
>
> Otherwise formally looks fine.
Thanks for looking at the patch.
Heiko
^ permalink raw reply
* [PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
From: Maxime Ripard @ 2016-10-10 12:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475828757-926-4-git-send-email-clabbe.montjoie@gmail.com>
On Fri, Oct 07, 2016 at 10:25:50AM +0200, Corentin Labbe wrote:
> This patch adds documentation for Device-Tree bindings for the
> Allwinner sun8i-emac driver.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> .../bindings/net/allwinner,sun8i-emac.txt | 70 ++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
>
> diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> new file mode 100644
> index 0000000..92e4ef3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> @@ -0,0 +1,70 @@
> +* Allwinner sun8i EMAC ethernet controller
> +
> +Required properties:
> +- compatible: should be one of the following string:
> + "allwinner,sun8i-a83t-emac"
> + "allwinner,sun8i-h3-emac"
> + "allwinner,sun50i-a64-emac"
> +- reg: address and length of the register for the device.
> +- syscon: A phandle to the syscon of the SoC
> +- interrupts: interrupt for the device
> +- clocks: A phandle to the reference clock for this device
> +- clock-names: should be "ahb"
> +- resets: A phandle to the reset control for this device
> +- reset-names: should be "ahb"
> +- phy-mode: See ethernet.txt
> +- phy-handle: See ethernet.txt
> +- #address-cells: shall be 1
> +- #size-cells: shall be 0
> +
> +Optional properties:
> +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. Default is 0)
> +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. Default is 0)
> +Both delay properties does not have units, there are arbitrary value.
> +The TX/RX clock delay chain settings are board specific and could be found
> +in vendor FEX files.
> +
> +Optional properties for "allwinner,sun8i-h3-emac":
> +- allwinner,leds-active-low: EPHY LEDs are active low
> +
> +Required child node of emac:
> +- mdio bus node: should be named mdio
> +
> +Required properties of the mdio node:
> +- #address-cells: shall be 1
> +- #size-cells: shall be 0
> +
> +The device node referenced by "phy" or "phy-handle" should be a child node
> +of the mdio node. See phy.txt for the generic PHY bindings.
> +
> +Required properties of the phy node with "allwinner,sun8i-h3-emac":
> +- clocks: an extra phandle to the reference clock for the EPHY
> +- resets: an extra phandle to the reset control for the EPHY
> +
> +Example:
> +
> +emac: ethernet at 01c0b000 {
> + compatible = "allwinner,sun8i-h3-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "ahb";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "ahb";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy = <&int_mii_phy>;
> + phy-mode = "mii";
> + allwinner,leds-active-low;
> + mdio: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy at 1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
That works for me, let's see how the DT maintainers feel about it.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161010/ffac48c2/attachment.sig>
^ permalink raw reply
* [PATCH v4 10/10] ARM: sunxi: Enable sun8i-emac driver on multi_v7_defconfig
From: Maxime Ripard @ 2016-10-10 12:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475828757-926-11-git-send-email-clabbe.montjoie@gmail.com>
On Fri, Oct 07, 2016 at 10:25:57AM +0200, Corentin Labbe wrote:
> Enable the sun8i-emac driver in the multi_v7 default configuration
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> arch/arm/configs/multi_v7_defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 5845910..f44d633 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -229,6 +229,7 @@ CONFIG_NETDEVICES=y
> CONFIG_VIRTIO_NET=y
> CONFIG_HIX5HD2_GMAC=y
> CONFIG_SUN4I_EMAC=y
> +CONFIG_SUN8I_EMAC=y
Any reason to build it statically?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161010/037cdfcf/attachment.sig>
^ permalink raw reply
* [PATCH v4 04/10] ARM: dts: sun8i-h3: Add dt node for the syscon control module
From: Maxime Ripard @ 2016-10-10 12:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475828757-926-5-git-send-email-clabbe.montjoie@gmail.com>
Hi,
On Fri, Oct 07, 2016 at 10:25:51AM +0200, Corentin Labbe wrote:
> This patch add the dt node for the syscon register present on the
> Allwinner H3.
>
> Only two register are present in this syscon and the only one useful is
> the one dedicated to EMAC clock.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> arch/arm/boot/dts/sun8i-h3.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index 8a95e36..1101d2f 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -140,6 +140,11 @@
> #size-cells = <1>;
> ranges;
>
> + syscon: syscon at 01c00000 {
> + compatible = "syscon";
It would be great to have a more specific compatible here in addition
to the syscon, like "allwinner,sun8i-h3-system-controller".
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161010/11087e9d/attachment-0001.sig>
^ permalink raw reply
* [PATCH v4 10/10] ARM: sunxi: Enable sun8i-emac driver on multi_v7_defconfig
From: LABBE Corentin @ 2016-10-10 12:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010123046.GH3462@lukather>
On Mon, Oct 10, 2016 at 02:30:46PM +0200, Maxime Ripard wrote:
> On Fri, Oct 07, 2016 at 10:25:57AM +0200, Corentin Labbe wrote:
> > Enable the sun8i-emac driver in the multi_v7 default configuration
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> > arch/arm/configs/multi_v7_defconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> > index 5845910..f44d633 100644
> > --- a/arch/arm/configs/multi_v7_defconfig
> > +++ b/arch/arm/configs/multi_v7_defconfig
> > @@ -229,6 +229,7 @@ CONFIG_NETDEVICES=y
> > CONFIG_VIRTIO_NET=y
> > CONFIG_HIX5HD2_GMAC=y
> > CONFIG_SUN4I_EMAC=y
> > +CONFIG_SUN8I_EMAC=y
>
> Any reason to build it statically?
>
No, just copied the same than CONFIG_SUN4I_EMAC that probably do not need it also.
Regards
Corentin Labbe
^ permalink raw reply
* [PATCH V3 0/8] IOMMU probe deferral support
From: Marek Szyprowski @ 2016-10-10 12:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475600632-21289-1-git-send-email-sricharan@codeaurora.org>
Hi Sricharan,
On 2016-10-04 19:03, Sricharan R wrote:
> Initial post from Laurent Pinchart[1]. This is
> series calls the dma ops configuration for the devices
> at a generic place so that it works for all busses.
> The dma_configure_ops for a device is now called during
> the device_attach callback just before the probe of the
> bus/driver is called. Similarly dma_deconfigure is called during
> device/driver_detach path.
>
>
> pci_bus_add_devices (platform/amba)(_device_create/driver_register)
> | |
> pci_bus_add_device (device_add/driver_register)
> | |
> device_attach device_initial_probe
> | |
> __device_attach_driver __device_attach_driver
> |
> driver_probe_device
> |
> really_probe
> |
> dma_configure
>
> Similarly on the device/driver_unregister path __device_release_driver is
> called which inturn calls dma_deconfigure.
>
> If the ACPI bus code follows the same, we can add acpi_dma_configure
> at the same place as of_dma_configure.
>
> This series is based on the recently merged Generic DT bindings for
> PCI IOMMUs and ARM SMMU from Robin Murphy robin.murphy at arm.com [2]
>
> This time tested this with platform and pci device for probe deferral
> and reprobe on arm64 based platform. There is an issue on the cleanup
> path for arm64 though, where there is WARN_ON if the dma_ops is reset while
> device is attached to an domain in arch_teardown_dma_ops.
> But with iommu_groups created from the iommu driver, the device is always
> attached to a domain/default_domain. So so the WARN has to be removed/handled
> probably.
Thanks for continuing work on this feature! Your can add my:
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
It works fine with Exynos SYSMMU driver, although a patch is needed to fix
infinite loop due to list corruption (same element is added twice if master
device fails with deferred probe):
From: Marek Szyprowski <m.szyprowski@samsung.com>
Date: Mon, 10 Oct 2016 14:22:42 +0200
Subject: [PATCH] iommu/exynos: ensure that sysmmu is added only once to its
master
Since adding IOMMU deferred probing support, of_xlate() callback might
be called more than once for given master device (for example it happens
when masters device driver fails with EPROBE_DEFER), so ensure that
SYSMMU controller is added to its master device (owner) only once.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/iommu/exynos-iommu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 30808e91b775..1525a86eb829 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1253,7 +1253,7 @@ static int exynos_iommu_of_xlate(struct device *dev,
{
struct exynos_iommu_owner *owner = dev->archdata.iommu;
struct platform_device *sysmmu = of_find_device_by_node(spec->np);
- struct sysmmu_drvdata *data;
+ struct sysmmu_drvdata *data, *entry;
if (!sysmmu)
return -ENODEV;
@@ -1271,6 +1271,10 @@ static int exynos_iommu_of_xlate(struct device *dev,
dev->archdata.iommu = owner;
}
+ list_for_each_entry(entry, &owner->controllers, owner_node)
+ if (entry == data)
+ return 0;
+
list_add_tail(&data->owner_node, &owner->controllers);
return 0;
}
--
1.9.1
>
> Previous post of this series [3].
>
> [V3]
> * Removed the patch to split dma_masks/dma_ops configuration separately
> based on review comments that both masks and ops are required only
> during the device probe time.
>
> * Reworked the series based on Generic DT bindings series [2].
>
> * Added call to iommu's remove_device in the cleanup path for arm and arm64.
>
> * Removed the notifier trick in arm64 to handle early device registration.
>
> * Added reset of dma_ops in cleanup path for arm based on comments.
>
> * Fixed the pci_iommu_configure path and tested with PCI device as well.
>
> * Fixed a bug to return the correct iommu_ops from patch 7 [4] in last post.
>
> * Fixed few other cosmetic comments.
>
> [V2]
> * Updated the Initial post to call dma_configure/deconfigure from generic code
>
> * Added iommu add_device callback from of_iommu_configure path
>
> [V1]
> * Initial post
>
> [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html
> [2] http://www.spinics.net/lists/devicetree/msg142943.html
> [3] https://www.mail-archive.com/iommu at lists.linux-foundation.org/msg13941.html
> [4] https://www.mail-archive.com/iommu at lists.linux-foundation.org/msg13940.html
>
>
>
> Laurent Pinchart (4):
> arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()
> of: dma: Move range size workaround to of_dma_get_range()
> of: dma: Make of_dma_deconfigure() public
> iommu: of: Handle IOMMU lookup failure with deferred probing or error
>
> Sricharan R (4):
> drivers: platform: Configure dma operations at probe time
> arm: dma-mapping: Reset the device's dma_ops
> arm/arm64: dma-mapping: Call iommu's remove_device callback during
> device detach
> arm64: dma-mapping: Remove the notifier trick to handle early setting
> of dma_ops
>
> arch/arm/mm/dma-mapping.c | 18 ++++++++
> arch/arm64/mm/dma-mapping.c | 107 +++++---------------------------------------
> drivers/base/dd.c | 10 +++++
> drivers/base/dma-mapping.c | 11 +++++
> drivers/iommu/of_iommu.c | 47 +++++++++++++++++--
> drivers/of/address.c | 20 ++++++++-
> drivers/of/device.c | 34 +++++++-------
> drivers/of/platform.c | 9 ----
> drivers/pci/probe.c | 5 +--
> include/linux/dma-mapping.h | 3 ++
> include/linux/of_device.h | 7 ++-
> 11 files changed, 138 insertions(+), 133 deletions(-)
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox