2019-05-09 21:57:08

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH 1/2] of/fdt: Remove dead code and mark functions with __init

Some functions in here are never called, and others are only called
during __init. Remove the dead code and some dead exports for functions
that don't exist (I'm looking at you of_fdt_get_string!). Mark some
functions with __init so we can throw them away after we boot up and
poke at the FDT blob too.

Cc: Hsin-Yi Wang <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../devicetree/bindings/common-properties.txt | 4 +-
drivers/of/fdt.c | 37 +++----------------
include/linux/of_fdt.h | 11 ------
3 files changed, 7 insertions(+), 45 deletions(-)

diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
index a3448bfa1c82..1c50d8700ab5 100644
--- a/Documentation/devicetree/bindings/common-properties.txt
+++ b/Documentation/devicetree/bindings/common-properties.txt
@@ -25,8 +25,8 @@ Optional properties:
If a binding supports these properties, then the binding should also
specify the default behavior if none of these properties are present.
In such cases, little-endian is the preferred default, but it is not
-a requirement. The of_device_is_big_endian() and of_fdt_is_big_endian()
-helper functions do assume that little-endian is the default, because
+a requirement. The of_device_is_big_endian()
+helper function assumes that little-endian is the default, because
most existing (PCI-based) drivers implicitly default to LE by using
readl/writel for MMIO accesses.

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index de893c9616a1..918098c9f72a 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -38,7 +38,7 @@
* memory entries in the /memory node. This function may be called
* any time after initial_boot_param is set.
*/
-void of_fdt_limit_memory(int limit)
+void __init of_fdt_limit_memory(int limit)
{
int memory;
int len;
@@ -110,25 +110,6 @@ static int of_fdt_is_compatible(const void *blob,
return 0;
}

-/**
- * of_fdt_is_big_endian - Return true if given node needs BE MMIO accesses
- * @blob: A device tree blob
- * @node: node to test
- *
- * Returns true if the node has a "big-endian" property, or if the kernel
- * was compiled for BE *and* the node has a "native-endian" property.
- * Returns false otherwise.
- */
-bool of_fdt_is_big_endian(const void *blob, unsigned long node)
-{
- if (fdt_getprop(blob, node, "big-endian", NULL))
- return true;
- if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) &&
- fdt_getprop(blob, node, "native-endian", NULL))
- return true;
- return false;
-}
-
static bool of_fdt_device_is_available(const void *blob, unsigned long node)
{
const char *status = fdt_getprop(blob, node, "status", NULL);
@@ -145,8 +126,8 @@ static bool of_fdt_device_is_available(const void *blob, unsigned long node)
/**
* of_fdt_match - Return true if node matches a list of compatible values
*/
-int of_fdt_match(const void *blob, unsigned long node,
- const char *const *compat)
+static int __init of_fdt_match(const void *blob, unsigned long node,
+ const char *const *compat)
{
unsigned int tmp, score = 0;

@@ -758,7 +739,7 @@ int __init of_scan_flat_dt_subnodes(unsigned long parent,
* @return offset of the subnode, or -FDT_ERR_NOTFOUND if there is none
*/

-int of_get_flat_dt_subnode_by_name(unsigned long node, const char *uname)
+int __init of_get_flat_dt_subnode_by_name(unsigned long node, const char *uname)
{
return fdt_subnode_offset(initial_boot_params, node, uname);
}
@@ -771,14 +752,6 @@ unsigned long __init of_get_flat_dt_root(void)
return 0;
}

-/**
- * of_get_flat_dt_size - Return the total size of the FDT
- */
-int __init of_get_flat_dt_size(void)
-{
- return fdt_totalsize(initial_boot_params);
-}
-
/**
* of_get_flat_dt_prop - Given a node in the flat blob, return the property ptr
*
@@ -804,7 +777,7 @@ int __init of_flat_dt_is_compatible(unsigned long node, const char *compat)
/**
* of_flat_dt_match - Return true if node matches a list of compatible values
*/
-int __init of_flat_dt_match(unsigned long node, const char *const *compat)
+static int __init of_flat_dt_match(unsigned long node, const char *const *compat)
{
return of_fdt_match(initial_boot_params, node, compat);
}
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index a713e5d156d8..acf820e88952 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -23,15 +23,6 @@
struct device_node;

/* For scanning an arbitrary device-tree at any time */
-extern char *of_fdt_get_string(const void *blob, u32 offset);
-extern void *of_fdt_get_property(const void *blob,
- unsigned long node,
- const char *name,
- int *size);
-extern bool of_fdt_is_big_endian(const void *blob,
- unsigned long node);
-extern int of_fdt_match(const void *blob, unsigned long node,
- const char *const *compat);
extern void *of_fdt_unflatten_tree(const unsigned long *blob,
struct device_node *dad,
struct device_node **mynodes);
@@ -64,9 +55,7 @@ extern int of_get_flat_dt_subnode_by_name(unsigned long node,
extern const void *of_get_flat_dt_prop(unsigned long node, const char *name,
int *size);
extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
-extern int of_flat_dt_match(unsigned long node, const char *const *matches);
extern unsigned long of_get_flat_dt_root(void);
-extern int of_get_flat_dt_size(void);
extern uint32_t of_get_flat_dt_phandle(unsigned long node);

extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
--
Sent by a computer through tubes


2019-05-14 17:08:49

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/2] of/fdt: Remove dead code and mark functions with __init

On Thu, May 9, 2019 at 4:55 PM Stephen Boyd <[email protected]> wrote:
>
> Some functions in here are never called, and others are only called
> during __init. Remove the dead code and some dead exports for functions
> that don't exist (I'm looking at you of_fdt_get_string!). Mark some
> functions with __init so we can throw them away after we boot up and
> poke at the FDT blob too.

Some of these aren't called, but there are cases where they could be
used instead of accessing initial_boot_params directly. The question
is what direction do we want to go. Make initial_boot_params private
or do away with the wrapper api and expect users to use libfdt api
only. I guess I lean toward the latter because that aligns with my
goal to move all the code in drivers/of/ somewhere else so I have
nothing to maintain. :)

>
> Cc: Hsin-Yi Wang <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../devicetree/bindings/common-properties.txt | 4 +-
> drivers/of/fdt.c | 37 +++----------------
> include/linux/of_fdt.h | 11 ------
> 3 files changed, 7 insertions(+), 45 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
> index a3448bfa1c82..1c50d8700ab5 100644
> --- a/Documentation/devicetree/bindings/common-properties.txt
> +++ b/Documentation/devicetree/bindings/common-properties.txt
> @@ -25,8 +25,8 @@ Optional properties:
> If a binding supports these properties, then the binding should also
> specify the default behavior if none of these properties are present.
> In such cases, little-endian is the preferred default, but it is not
> -a requirement. The of_device_is_big_endian() and of_fdt_is_big_endian()
> -helper functions do assume that little-endian is the default, because
> +a requirement. The of_device_is_big_endian()

Hum, we shouldn't have kernel functions in binding docs. Can you
reword to remove both functions.

> +helper function assumes that little-endian is the default, because
> most existing (PCI-based) drivers implicitly default to LE by using
> readl/writel for MMIO accesses.
>

2019-05-14 20:24:34

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 1/2] of/fdt: Remove dead code and mark functions with __init

Quoting Rob Herring (2019-05-14 10:06:48)
> On Thu, May 9, 2019 at 4:55 PM Stephen Boyd <[email protected]> wrote:
> > diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
> > index a3448bfa1c82..1c50d8700ab5 100644
> > --- a/Documentation/devicetree/bindings/common-properties.txt
> > +++ b/Documentation/devicetree/bindings/common-properties.txt
> > @@ -25,8 +25,8 @@ Optional properties:
> > If a binding supports these properties, then the binding should also
> > specify the default behavior if none of these properties are present.
> > In such cases, little-endian is the preferred default, but it is not
> > -a requirement. The of_device_is_big_endian() and of_fdt_is_big_endian()
> > -helper functions do assume that little-endian is the default, because
> > +a requirement. The of_device_is_big_endian()
>
> Hum, we shouldn't have kernel functions in binding docs. Can you
> reword to remove both functions.
>

Ok. I'll resend with this sentence cleaned up more.