On Thu, 2015-26-02 at 00:04:47 UTC, Dave Olson wrote:
> @@ -324,14 +335,33 @@ static bool cache_node_is_unified(const struct
> device_node *np)
> return of_get_property(np, "cache-unified", NULL);
> }
>
> +/*
> + * Handle unified caches that have two different types of tags. Most
> embedded
> + * use cache-size, etc. for the unified cache size, but open firmware systems
> + * use d-cache-size, etc. Since they all appear to be consistent, check on
> + * initialization for which type we are, and use the appropriate structure.
> + */
> static struct cache *cache_do_one_devnode_unified(struct device_node *node,
> int level)
> {
> struct cache *cache;
> + int ucache;
>
> pr_debug("creating L%d ucache for %s\n", level, node->full_name);
>
> cache = new_cache(CACHE_TYPE_UNIFIED, level, node);
^^
> + if (of_get_property(node,
> + cache_type_info[CACHE_TYPE_UNIFIED_D].size_prop, NULL)) {
> + ucache = CACHE_TYPE_UNIFIED_D;
> + } else {
> + ucache = CACHE_TYPE_UNIFIED; /* assume embedded */
> + if (of_get_property(node,
> + cache_type_info[CACHE_TYPE_UNIFIED].size_prop, NULL) ==
> + NULL)
> + printk(KERN_WARNING "Unified cache property missing\n");
> + }
> +
> + cache = new_cache(ucache, level, node);
^^
>
> return cache;
> }
That looks fishy. You create a cache, and then throw it away and create another
one and return that. I don't think that's what you intended, is it?
It would also be cleaner I think if you created another helper, eg.
cache_is_unified_d() to do the property lookup.
And also I don't think you need to do the second property lookup, especially if
all you're going to do is print a warning.
cheers
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
