2014-02-11 16:26:33

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH 0/8] of_find_matching_node/of_match_node -> of_find_matching_node_and_match

This patchset updates users of the of_find_matching_node()/of_match_node() pair
to leverage of_find_matching_node_and_match(), which only requires one walk
through the match table. This functionality was added by Stephen Warren[1], and
landed in v3.8.

Cc: Stephen Warren <[email protected]>
Cc: Laurent Pinchart <[email protected]>

1: 50c8af4cf9 (of: introduce for_each_matching_node_and_match())

Josh Cartwright (8):
bus: arm-cci: make use of of_find_matching_node_and_match
bus: mvebu-mbus: make use of of_find_matching_node_and_match
ARM: at91: make use of of_find_matching_node_and_match
ARM: mvebu: make use of of_find_matching_node_and_match
ARM: prima2: make use of of_find_matching_node_and_match
ARM: l2x0: make use of of_find_matching_node_and_match
C6X: make use of of_find_matching_node_and_match
cpufreq: ppc: make use of of_find_matching_node_and_match

arch/arm/mach-at91/setup.c | 16 ++++------------
arch/arm/mach-mvebu/system-controller.c | 9 ++++-----
arch/arm/mach-prima2/l2x0.c | 7 ++++---
arch/arm/mm/cache-l2x0.c | 5 +++--
arch/c6x/platforms/plldata.c | 7 +++----
drivers/bus/arm-cci.c | 5 +++--
drivers/bus/mvebu-mbus.c | 3 +--
drivers/cpufreq/ppc-corenet-cpufreq.c | 3 +--
8 files changed, 23 insertions(+), 32 deletions(-)

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation


2014-02-11 16:26:39

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH 5/8] ARM: prima2: make use of of_find_matching_node_and_match

Instead of the of_find_matching_node()/of_match_node() pair, which requires two
iterations through the match table, make use of of_find_matching_node_and_match(),
which only iterates through the table once.

While we're here, mark the prima2_l2x0 table const.

Signed-off-by: Josh Cartwright <[email protected]>
---
arch/arm/mach-prima2/l2x0.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-prima2/l2x0.c b/arch/arm/mach-prima2/l2x0.c
index cbcbe9c..982b0b6 100644
--- a/arch/arm/mach-prima2/l2x0.c
+++ b/arch/arm/mach-prima2/l2x0.c
@@ -28,7 +28,7 @@ static struct l2x0_aux marco_l2x0_aux __initconst = {
.mask = L2X0_AUX_CTRL_MASK,
};

-static struct of_device_id sirf_l2x0_ids[] __initconst = {
+static const struct of_device_id sirf_l2x0_ids[] __initconst = {
{ .compatible = "sirf,prima2-pl310-cache", .data = &prima2_l2x0_aux, },
{ .compatible = "sirf,marco-pl310-cache", .data = &marco_l2x0_aux, },
{},
@@ -36,12 +36,13 @@ static struct of_device_id sirf_l2x0_ids[] __initconst = {

static int __init sirfsoc_l2x0_init(void)
{
+ const struct of_device_id *match;
struct device_node *np;
const struct l2x0_aux *aux;

- np = of_find_matching_node(NULL, sirf_l2x0_ids);
+ np = of_find_matching_node_and_match(NULL, sirf_l2x0_ids, &match);
if (np) {
- aux = of_match_node(sirf_l2x0_ids, np)->data;
+ aux = match->data;
return l2x0_of_init(aux->val, aux->mask);
}

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 16:26:37

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH 8/8] cpufreq: ppc: make use of of_find_matching_node_and_match

Instead of the of_find_matching_node()/of_match_node() pair, which requires two
iterations through the match table, make use of of_find_matching_node_and_match(),
which only iterates through the table once.

Signed-off-by: Josh Cartwright <[email protected]>
---
drivers/cpufreq/ppc-corenet-cpufreq.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
index 051000f..041ef3a 100644
--- a/drivers/cpufreq/ppc-corenet-cpufreq.c
+++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
@@ -278,7 +278,7 @@ static int __init ppc_corenet_cpufreq_init(void)
const struct soc_data *data;
unsigned int cpu;

- np = of_find_matching_node(NULL, node_matches);
+ np = of_find_matching_node_and_match(NULL, node_matches, &match);
if (!np)
return -ENODEV;

@@ -288,7 +288,6 @@ static int __init ppc_corenet_cpufreq_init(void)
cpumask_copy(per_cpu(cpu_mask, cpu), cpu_core_mask(cpu));
}

- match = of_match_node(node_matches, np);
data = match->data;
if (data) {
if (data->flag)
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 16:26:35

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH 3/8] ARM: at91: make use of of_find_matching_node_and_match

Instead of the of_find_matching_node()/of_match_node() pair, which requires two
iterations through the match table, make use of of_find_matching_node_and_match(),
which only iterates through the table once.

While we're here, mark the rtsc id table const.

Signed-off-by: Josh Cartwright <[email protected]>
---
arch/arm/mach-at91/setup.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c
index f7ca97b..e884de8 100644
--- a/arch/arm/mach-at91/setup.c
+++ b/arch/arm/mach-at91/setup.c
@@ -352,7 +352,7 @@ void __init at91_ioremap_matrix(u32 base_addr)
}

#if defined(CONFIG_OF)
-static struct of_device_id rstc_ids[] = {
+static const struct of_device_id rstc_ids[] = {
{ .compatible = "atmel,at91sam9260-rstc", .data = at91sam9_alt_restart },
{ .compatible = "atmel,at91sam9g45-rstc", .data = at91sam9g45_restart },
{ /*sentinel*/ }
@@ -363,7 +363,7 @@ static void at91_dt_rstc(void)
struct device_node *np;
const struct of_device_id *of_id;

- np = of_find_matching_node(NULL, rstc_ids);
+ np = of_find_matching_node_and_match(NULL, rstc_ids, &of_id);
if (!np)
panic("unable to find compatible rstc node in dtb\n");

@@ -371,10 +371,6 @@ static void at91_dt_rstc(void)
if (!at91_rstc_base)
panic("unable to map rstc cpu registers\n");

- of_id = of_match_node(rstc_ids, np);
- if (!of_id)
- panic("AT91: rtsc no restart function available\n");
-
arm_pm_restart = of_id->data;

of_node_put(np);
@@ -392,7 +388,7 @@ static void at91_dt_ramc(void)
struct device_node *np;
const struct of_device_id *of_id;

- np = of_find_matching_node(NULL, ramc_ids);
+ np = of_find_matching_node_and_match(NULL, ramc_ids, &of_id);
if (!np)
panic("unable to find compatible ram controller node in dtb\n");

@@ -402,11 +398,7 @@ static void at91_dt_ramc(void)
/* the controller may have 2 banks */
at91_ramc_base[1] = of_iomap(np, 1);

- of_id = of_match_node(ramc_ids, np);
- if (!of_id)
- pr_warn("AT91: ramc no standby function available\n");
- else
- at91_pm_set_standby(of_id->data);
+ at91_pm_set_standby(of_id->data);

of_node_put(np);
}
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 16:26:32

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH 1/8] bus: arm-cci: make use of of_find_matching_node_and_match

Instead of the of_find_matching_node()/of_match_node() pair, which requires two
iterations through the match table, make use of of_find_matching_node_and_match(),
which only iterates through the table once.

This also has the side effect of fixing the following following error hit
during randconfig testing:

drivers/bus/arm-cci.c: In function ‘cci_probe’:
drivers/bus/arm-cci.c:976:49: warning: dereferencing ‘void *’ pointer [enabled by default]
cci_config = of_match_node(arm_cci_matches, np)->data;
^
drivers/bus/arm-cci.c:976:49: error: request for member ‘data’ in something not a structure or union

Signed-off-by: Josh Cartwright <[email protected]>
---
drivers/bus/arm-cci.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 962fd35..e249cdb2 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -962,6 +962,7 @@ static const struct of_device_id arm_cci_ctrl_if_matches[] = {

static int cci_probe(void)
{
+ const struct of_device_id *match;
struct cci_nb_ports const *cci_config;
int ret, i, nb_ace = 0, nb_ace_lite = 0;
struct device_node *np, *cp;
@@ -969,11 +970,11 @@ static int cci_probe(void)
const char *match_str;
bool is_ace;

- np = of_find_matching_node(NULL, arm_cci_matches);
+ np = of_find_matching_node_and_match(NULL, arm_cci_matches, &match);
if (!np)
return -ENODEV;

- cci_config = of_match_node(arm_cci_matches, np)->data;
+ cci_config = match->data;
if (!cci_config)
return -ENODEV;

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 16:26:30

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH 2/8] bus: mvebu-mbus: make use of of_find_matching_node_and_match

Instead of the of_find_matching_node()/of_match_node() pair, which requires two
iterations through the match table, make use of of_find_matching_node_and_match(),
which only iterates through the table once.

Signed-off-by: Josh Cartwright <[email protected]>
---
drivers/bus/mvebu-mbus.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index 725c461..31f2c80 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -890,13 +890,12 @@ int __init mvebu_mbus_dt_init(void)
const __be32 *prop;
int ret;

- np = of_find_matching_node(NULL, of_mvebu_mbus_ids);
+ np = of_find_matching_node_and_match(NULL, of_mvebu_mbus_ids, &of_id);
if (!np) {
pr_err("could not find a matching SoC family\n");
return -ENODEV;
}

- of_id = of_match_node(of_mvebu_mbus_ids, np);
mbus_state.soc = of_id->data;

prop = of_get_property(np, "controller", NULL);
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 16:34:04

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH 7/8] C6X: make use of of_find_matching_node_and_match

Instead of the of_find_matching_node()/of_match_node() pair, which requires two
iterations through the match table, make use of of_find_matching_node_and_match(),
which only iterates through the table once.

Signed-off-by: Josh Cartwright <[email protected]>
---
arch/c6x/platforms/plldata.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/c6x/platforms/plldata.c b/arch/c6x/platforms/plldata.c
index 755359e..8fddc46 100644
--- a/arch/c6x/platforms/plldata.c
+++ b/arch/c6x/platforms/plldata.c
@@ -397,7 +397,7 @@ static void __init c6678_setup_clocks(struct device_node *node)
}
#endif /* CONFIG_SOC_TMS320C6678 */

-static struct of_device_id c6x_clkc_match[] __initdata = {
+static const struct of_device_id c6x_clkc_match[] __initdata = {
#ifdef CONFIG_SOC_TMS320C6455
{ .compatible = "ti,c6455-pll", .data = c6455_setup_clocks },
#endif
@@ -426,7 +426,7 @@ void __init c64x_setup_clocks(void)
int err;
u32 val;

- node = of_find_matching_node(NULL, c6x_clkc_match);
+ node = of_find_matching_node_and_match(NULL, c6x_clkc_match, &id);
if (!node)
return;

@@ -458,8 +458,7 @@ void __init c64x_setup_clocks(void)
pll->lock_delay = val;

/* id->data is a pointer to SoC-specific setup */
- id = of_match_node(c6x_clkc_match, node);
- if (id && id->data) {
+ if (id->data) {
__setup_clocks = id->data;
__setup_clocks(node);
}
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 16:34:46

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH 6/8] ARM: l2x0: make use of of_find_matching_node_and_match

Instead of the of_find_matching_node()/of_match_node() pair, which requires two
iterations through the match table, make use of of_find_matching_node_and_match(),
which only iterates through the table once.

Signed-off-by: Josh Cartwright <[email protected]>
---
arch/arm/mm/cache-l2x0.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 7abde2c..e35fff3 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -970,11 +970,12 @@ static const struct of_device_id l2x0_ids[] __initconst = {

int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
{
+ const struct of_device_id *match;
struct device_node *np;
const struct l2x0_of_data *data;
struct resource res;

- np = of_find_matching_node(NULL, l2x0_ids);
+ np = of_find_matching_node_and_match(NULL, l2x0_ids, &match);
if (!np)
return -ENODEV;

@@ -987,7 +988,7 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)

l2x0_saved_regs.phy_base = res.start;

- data = of_match_node(l2x0_ids, np)->data;
+ data = match->data;

/* L2 configuration can only be changed if the cache is disabled */
if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & L2X0_CTRL_EN)) {
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 16:35:02

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH 4/8] ARM: mvebu: make use of of_find_matching_node_and_match

Instead of the of_find_matching_node()/of_match_node() pair, which requires two
iterations through the match table, make use of of_find_matching_node_and_match(),
which only iterates through the table once.

While we're here, mark the of_system_controller table const.

Signed-off-by: Josh Cartwright <[email protected]>
---
arch/arm/mach-mvebu/system-controller.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-mvebu/system-controller.c b/arch/arm/mach-mvebu/system-controller.c
index a7fb89a..e6e300a 100644
--- a/arch/arm/mach-mvebu/system-controller.c
+++ b/arch/arm/mach-mvebu/system-controller.c
@@ -54,7 +54,7 @@ static const struct mvebu_system_controller orion_system_controller = {
.system_soft_reset = 0x1,
};

-static struct of_device_id of_system_controller_table[] = {
+static const struct of_device_id of_system_controller_table[] = {
{
.compatible = "marvell,orion-system-controller",
.data = (void *) &orion_system_controller,
@@ -90,13 +90,12 @@ void mvebu_restart(enum reboot_mode mode, const char *cmd)

static int __init mvebu_system_controller_init(void)
{
+ const struct of_device_id *match;
struct device_node *np;

- np = of_find_matching_node(NULL, of_system_controller_table);
+ np = of_find_matching_node_and_match(NULL, of_system_controller_table,
+ &match);
if (np) {
- const struct of_device_id *match =
- of_match_node(of_system_controller_table, np);
- BUG_ON(!match);
system_controller_base = of_iomap(np, 0);
mvebu_sc = (struct mvebu_system_controller *)match->data;
of_node_put(np);
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 16:40:13

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 2/8] bus: mvebu-mbus: make use of of_find_matching_node_and_match

On Tue, Feb 11, 2014 at 10:24:00AM -0600, Josh Cartwright wrote:
> Instead of the of_find_matching_node()/of_match_node() pair, which requires two
> iterations through the match table, make use of of_find_matching_node_and_match(),
> which only iterates through the table once.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> drivers/bus/mvebu-mbus.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)

Acked-by: Jason Cooper <[email protected]>

thx,

Jason.

2014-02-11 16:41:11

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 4/8] ARM: mvebu: make use of of_find_matching_node_and_match

Dear Josh Cartwright,

On Tue, 11 Feb 2014 10:24:02 -0600, Josh Cartwright wrote:
> Instead of the of_find_matching_node()/of_match_node() pair, which requires two
> iterations through the match table, make use of of_find_matching_node_and_match(),
> which only iterates through the table once.
>
> While we're here, mark the of_system_controller table const.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> arch/arm/mach-mvebu/system-controller.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Thomas Petazzoni <[email protected]>
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-02-11 16:53:30

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 4/8] ARM: mvebu: make use of of_find_matching_node_and_match

On Tue, Feb 11, 2014 at 10:24:02AM -0600, Josh Cartwright wrote:
> Instead of the of_find_matching_node()/of_match_node() pair, which requires two
> iterations through the match table, make use of of_find_matching_node_and_match(),
> which only iterates through the table once.
>
> While we're here, mark the of_system_controller table const.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> arch/arm/mach-mvebu/system-controller.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-mvebu/system-controller.c b/arch/arm/mach-mvebu/system-controller.c
> index a7fb89a..e6e300a 100644
> --- a/arch/arm/mach-mvebu/system-controller.c
> +++ b/arch/arm/mach-mvebu/system-controller.c
> @@ -54,7 +54,7 @@ static const struct mvebu_system_controller orion_system_controller = {
> .system_soft_reset = 0x1,
> };
>
> -static struct of_device_id of_system_controller_table[] = {
> +static const struct of_device_id of_system_controller_table[] = {
> {
> .compatible = "marvell,orion-system-controller",
> .data = (void *) &orion_system_controller,
> @@ -90,13 +90,12 @@ void mvebu_restart(enum reboot_mode mode, const char *cmd)
>
> static int __init mvebu_system_controller_init(void)
> {
> + const struct of_device_id *match;
> struct device_node *np;
>
> - np = of_find_matching_node(NULL, of_system_controller_table);
> + np = of_find_matching_node_and_match(NULL, of_system_controller_table,
> + &match);
> if (np) {
> - const struct of_device_id *match =
> - of_match_node(of_system_controller_table, np);


> - BUG_ON(!match);

Gregory, is it ok to remove this? It was added with the original code
submission for mach-mvebu. mvebu_restart() will handle this
gracefully...

> system_controller_base = of_iomap(np, 0);
> mvebu_sc = (struct mvebu_system_controller *)match->data;
> of_node_put(np);

thx,

Jason.

2014-02-11 17:10:54

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 4/8] ARM: mvebu: make use of of_find_matching_node_and_match

Dear Jason Cooper,

On Tue, 11 Feb 2014 11:53:14 -0500, Jason Cooper wrote:

> > - np = of_find_matching_node(NULL, of_system_controller_table);
> > + np = of_find_matching_node_and_match(NULL, of_system_controller_table,
> > + &match);
> > if (np) {
> > - const struct of_device_id *match =
> > - of_match_node(of_system_controller_table, np);
>
>
> > - BUG_ON(!match);
>
> Gregory, is it ok to remove this? It was added with the original code
> submission for mach-mvebu. mvebu_restart() will handle this
> gracefully...

The BUG_ON here can normally never be reached. If
of_find_matching_node() returns a non-NULL result, then of_match_node()
should also return a non-NULL result.

Or I'm missing something :)

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-02-11 17:31:44

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH 4/8] ARM: mvebu: make use of of_find_matching_node_and_match

On 11/02/2014 18:10, Thomas Petazzoni wrote:
> Dear Jason Cooper,
>
> On Tue, 11 Feb 2014 11:53:14 -0500, Jason Cooper wrote:
>
>>> - np = of_find_matching_node(NULL, of_system_controller_table);
>>> + np = of_find_matching_node_and_match(NULL, of_system_controller_table,
>>> + &match);
>>> if (np) {
>>> - const struct of_device_id *match =
>>> - of_match_node(of_system_controller_table, np);
>>
>>
>>> - BUG_ON(!match);
>>
>> Gregory, is it ok to remove this? It was added with the original code
>> submission for mach-mvebu. mvebu_restart() will handle this
>> gracefully...
>
> The BUG_ON here can normally never be reached. If
> of_find_matching_node() returns a non-NULL result, then of_match_node()
> should also return a non-NULL result.
>
> Or I'm missing something :)

No you're almost right!

The only case we can get it, would be if we were declaring something like:

static struct of_device_id of_system_controller_table[] = {
{
.compatible = "foo,bar-controller",
},
[...]

instead of

static struct of_device_id of_system_controller_table[] = {
{
.compatible = "foo,bar",
.data = (void *) &bar_controller,
},
[...]

This test is very paranoid, so I agree to remove it.


Acked-by: Gregory CLEMENT <[email protected]>


Thanks,

Gregory


>
> Thomas
>


--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2014-02-11 17:35:04

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 4/8] ARM: mvebu: make use of of_find_matching_node_and_match

On Tue, Feb 11, 2014 at 06:31:33PM +0100, Gregory CLEMENT wrote:
> On 11/02/2014 18:10, Thomas Petazzoni wrote:
> > Dear Jason Cooper,
> >
> > On Tue, 11 Feb 2014 11:53:14 -0500, Jason Cooper wrote:
> >
> >>> - np = of_find_matching_node(NULL, of_system_controller_table);
> >>> + np = of_find_matching_node_and_match(NULL, of_system_controller_table,
> >>> + &match);
> >>> if (np) {
> >>> - const struct of_device_id *match =
> >>> - of_match_node(of_system_controller_table, np);
> >>
> >>
> >>> - BUG_ON(!match);
> >>
> >> Gregory, is it ok to remove this? It was added with the original code
> >> submission for mach-mvebu. mvebu_restart() will handle this
> >> gracefully...
> >
> > The BUG_ON here can normally never be reached. If
> > of_find_matching_node() returns a non-NULL result, then of_match_node()
> > should also return a non-NULL result.
> >
> > Or I'm missing something :)
>
> No you're almost right!
>
> The only case we can get it, would be if we were declaring something like:
>
> static struct of_device_id of_system_controller_table[] = {
> {
> .compatible = "foo,bar-controller",
> },
> [...]
>
> instead of
>
> static struct of_device_id of_system_controller_table[] = {
> {
> .compatible = "foo,bar",
> .data = (void *) &bar_controller,
> },
> [...]
>
> This test is very paranoid, so I agree to remove it.
>
>
> Acked-by: Gregory CLEMENT <[email protected]>

Ok, great! Josh, do you want us to take the two mvebu patches through
mvebu/arm-soc? Or would you prefer to take them?

thx,

Jason.

2014-02-11 17:46:05

by Josh Cartwright

[permalink] [raw]
Subject: Re: [PATCH 4/8] ARM: mvebu: make use of of_find_matching_node_and_match

On Tue, Feb 11, 2014 at 12:34:46PM -0500, Jason Cooper wrote:
>
> Ok, great! Josh, do you want us to take the two mvebu patches through
> mvebu/arm-soc? Or would you prefer to take them?

Please, take them through the mvebu tree.

Thanks,
Josh

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 17:46:43

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH 3/8] ARM: at91: make use of of_find_matching_node_and_match

On 11/02/2014 17:24, Josh Cartwright :
> Instead of the of_find_matching_node()/of_match_node() pair, which requires two
> iterations through the match table, make use of of_find_matching_node_and_match(),
> which only iterates through the table once.
>
> While we're here, mark the rtsc id table const.

Well, I might remove this one, just because other id tables are not
marked as "const" in the same file... So it can be good to change all of
them in a raw.


> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> arch/arm/mach-at91/setup.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c
> index f7ca97b..e884de8 100644
> --- a/arch/arm/mach-at91/setup.c
> +++ b/arch/arm/mach-at91/setup.c
> @@ -352,7 +352,7 @@ void __init at91_ioremap_matrix(u32 base_addr)
> }
>
> #if defined(CONFIG_OF)
> -static struct of_device_id rstc_ids[] = {
> +static const struct of_device_id rstc_ids[] = {
> { .compatible = "atmel,at91sam9260-rstc", .data = at91sam9_alt_restart },
> { .compatible = "atmel,at91sam9g45-rstc", .data = at91sam9g45_restart },
> { /*sentinel*/ }
> @@ -363,7 +363,7 @@ static void at91_dt_rstc(void)
> struct device_node *np;
> const struct of_device_id *of_id;
>
> - np = of_find_matching_node(NULL, rstc_ids);
> + np = of_find_matching_node_and_match(NULL, rstc_ids, &of_id);
> if (!np)
> panic("unable to find compatible rstc node in dtb\n");
>
> @@ -371,10 +371,6 @@ static void at91_dt_rstc(void)
> if (!at91_rstc_base)
> panic("unable to map rstc cpu registers\n");
>
> - of_id = of_match_node(rstc_ids, np);
> - if (!of_id)
> - panic("AT91: rtsc no restart function available\n");
> -
> arm_pm_restart = of_id->data;
>
> of_node_put(np);
> @@ -392,7 +388,7 @@ static void at91_dt_ramc(void)
> struct device_node *np;
> const struct of_device_id *of_id;
>
> - np = of_find_matching_node(NULL, ramc_ids);
> + np = of_find_matching_node_and_match(NULL, ramc_ids, &of_id);
> if (!np)
> panic("unable to find compatible ram controller node in dtb\n");
>
> @@ -402,11 +398,7 @@ static void at91_dt_ramc(void)
> /* the controller may have 2 banks */
> at91_ramc_base[1] = of_iomap(np, 1);
>
> - of_id = of_match_node(ramc_ids, np);
> - if (!of_id)
> - pr_warn("AT91: ramc no standby function available\n");
> - else
> - at91_pm_set_standby(of_id->data);
> + at91_pm_set_standby(of_id->data);

Even if it changes the strict behavior of the function, I do not see any
advantage in keeping the pr_warn() path instead of a simple panic()
protecting the "find" and "match" together...

So, without the "const" modification, it ends up with a:

Acked-by: Nicolas Ferre <[email protected]>

=> if you want, I can take the patch with me through arm-soc with at91
material for 3.15 and complete your "const" modification. What do you
think about that?

>
> of_node_put(np);
> }
>

Thanks for having taking care of this file, bye,
--
Nicolas Ferre

2014-02-11 18:46:22

by Josh Cartwright

[permalink] [raw]
Subject: Re: [PATCH 3/8] ARM: at91: make use of of_find_matching_node_and_match

On Tue, Feb 11, 2014 at 06:46:28PM +0100, Nicolas Ferre wrote:
> On 11/02/2014 17:24, Josh Cartwright :
> > Instead of the of_find_matching_node()/of_match_node() pair, which requires two
> > iterations through the match table, make use of of_find_matching_node_and_match(),
> > which only iterates through the table once.
> >
> > While we're here, mark the rtsc id table const.
>
> Well, I might remove this one, just because other id tables are not
> marked as "const" in the same file... So it can be good to change all of
> them in a raw.

Indeed, I was only looking for a specific pattern.

[..]
> => if you want, I can take the patch with me through arm-soc with at91
> material for 3.15 and complete your "const" modification. What do you
> think about that?

Yes, this sounds good.

Thanks,
Josh

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2014-02-11 19:22:45

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 2/8] bus: mvebu-mbus: make use of of_find_matching_node_and_match

On Tue, Feb 11, 2014 at 10:24:00AM -0600, Josh Cartwright wrote:
> Instead of the of_find_matching_node()/of_match_node() pair, which requires two
> iterations through the match table, make use of of_find_matching_node_and_match(),
> which only iterates through the table once.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> drivers/bus/mvebu-mbus.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)

Applied to mvebu/drivers

thx,

Jason.

2014-02-11 19:29:50

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 4/8] ARM: mvebu: make use of of_find_matching_node_and_match

On Tue, Feb 11, 2014 at 10:24:02AM -0600, Josh Cartwright wrote:
> Instead of the of_find_matching_node()/of_match_node() pair, which requires two
> iterations through the match table, make use of of_find_matching_node_and_match(),
> which only iterates through the table once.
>
> While we're here, mark the of_system_controller table const.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> arch/arm/mach-mvebu/system-controller.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)

Applied to mvebu/soc with Thomas' Reviewed-by, and Gregory's Ack.

thx,

Jason.

2014-02-12 05:01:46

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 8/8] cpufreq: ppc: make use of of_find_matching_node_and_match

On 11 February 2014 21:54, Josh Cartwright <[email protected]> wrote:
> Instead of the of_find_matching_node()/of_match_node() pair, which requires two
> iterations through the match table, make use of of_find_matching_node_and_match(),
> which only iterates through the table once.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> ---
> drivers/cpufreq/ppc-corenet-cpufreq.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c
> index 051000f..041ef3a 100644
> --- a/drivers/cpufreq/ppc-corenet-cpufreq.c
> +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c
> @@ -278,7 +278,7 @@ static int __init ppc_corenet_cpufreq_init(void)
> const struct soc_data *data;
> unsigned int cpu;
>
> - np = of_find_matching_node(NULL, node_matches);
> + np = of_find_matching_node_and_match(NULL, node_matches, &match);
> if (!np)
> return -ENODEV;
>
> @@ -288,7 +288,6 @@ static int __init ppc_corenet_cpufreq_init(void)
> cpumask_copy(per_cpu(cpu_mask, cpu), cpu_core_mask(cpu));
> }
>
> - match = of_match_node(node_matches, np);
> data = match->data;
> if (data) {
> if (data->flag)

Acked-by: Viresh Kumar <[email protected]>