| Message ID | 20180830190523.31474-4-robh@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | of: root #{size,address}-cells clean-ups | expand |
On 05/09/18 13:12, Rob Herring wrote: > On Tue, Sep 4, 2018 at 11:59 PM Mark Cave-Ayland > <mark.cave-ayland@ilande.co.uk> wrote: >> >> On 05/09/18 02:55, Frank Rowand wrote: >> >>> On 08/30/18 12:05, Rob Herring wrote: >>>> Only some old OpenFirmware implementations rely on default sizes. Any >>>> FDT and modern implementation should have explicit properties. Make the >>>> OF_ROOT_NODE_*_CELLS_DEFAULT defines private so we don't get any outside >>>> users. >>>> >>>> This also gets us one step closer to removing the asm/prom.h dependency on >>>> Sparc. >> >> Just for the record: you say "any FDT and modern implementation should >> have explicit properties", however the default values of these >> properties when missing are clearly defined in the IEEE-1275 >> specification (Annex A): > > The spec may define defaults, but best practice is to be explicit. For > FDT, dtc doesn't allow defaults (and never has). No arch added in the > last 10 years has relied on the defaults. > >> "#address-cells" >> Standard property name to define the package’s address format. >> ... >> In a package with a "decode-unit" method, a missing "#address-cells" >> property signifies that the number of >> address cells is two. > > So only Sparc is compliant as the default for everyone else is 1. > >> "#size-cells" >> Standard property name to define the package’s address size format. >> ... >> A missing "#size-cells" property signifies the default value of one. >> >> I can't speak for FDT but it isn't completely unreasonable for a guest >> parsing a DT without these properties to assume these default values. > > I'm not removing the defaults. Just not allowing for code outside of > drivers/of/ to use the defaults. Got it - if dtc requires explicit properties, then I agree there should be no issues with this patch. Sorry for the noise. ATB, Mark.
diff --git a/arch/sparc/include/asm/prom.h b/arch/sparc/include/asm/prom.h index d955c8df62d6..1902db27ff4b 100644 --- a/arch/sparc/include/asm/prom.h +++ b/arch/sparc/include/asm/prom.h @@ -24,9 +24,6 @@ #include <linux/atomic.h> #include <linux/irqdomain.h> -#define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 2 -#define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1 - #define of_compat_cmp(s1, s2, l) strncmp((s1), (s2), (l)) #define of_prop_cmp(s1, s2) strcasecmp((s1), (s2)) #define of_node_cmp(s1, s2) strcmp((s1), (s2)) diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 216175d11d3d..5d1567025358 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -27,6 +27,14 @@ struct alias_prop { char stem[0]; }; +#if defined(CONFIG_SPARC) +#define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 2 +#else +#define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 1 +#endif + +#define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1 + extern struct mutex of_mutex; extern struct list_head aliases_lookup; extern struct kset *of_kset; diff --git a/include/linux/of.h b/include/linux/of.h index 506beca9588d..49d85f670c75 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -247,12 +247,6 @@ static inline unsigned long of_read_ulong(const __be32 *cell, int size) #include <asm/prom.h> #endif -/* Default #address and #size cells. Allow arch asm/prom.h to override */ -#if !defined(OF_ROOT_NODE_ADDR_CELLS_DEFAULT) -#define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 1 -#define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1 -#endif - #define OF_IS_DYNAMIC(x) test_bit(OF_DYNAMIC, &x->_flags) #define OF_MARK_DYNAMIC(x) set_bit(OF_DYNAMIC, &x->_flags)
Only some old OpenFirmware implementations rely on default sizes. Any FDT and modern implementation should have explicit properties. Make the OF_ROOT_NODE_*_CELLS_DEFAULT defines private so we don't get any outside users. This also gets us one step closer to removing the asm/prom.h dependency on Sparc. Cc: "David S. Miller" <davem@davemloft.net> Cc: Frank Rowand <frowand.list@gmail.com> Cc: sparclinux@vger.kernel.org Signed-off-by: Rob Herring <robh@kernel.org> --- arch/sparc/include/asm/prom.h | 3 --- drivers/of/of_private.h | 8 ++++++++ include/linux/of.h | 6 ------ 3 files changed, 8 insertions(+), 9 deletions(-) -- 2.17.1