* difference between fdtdec and fdt_support ? @ 2022-01-06 12:21 Marek Behún 2022-01-06 14:37 ` Rasmus Villemoes 2022-01-06 15:48 ` Simon Glass 0 siblings, 2 replies; 7+ messages in thread From: Marek Behún @ 2022-01-06 12:21 UTC (permalink / raw) To: Simon Glass; +Cc: U-Boot Mailing List Hi Simon, I am a little confused. We have common/fdt_support.c and lib/fdtdec.c The second one implements for example fdtdec_get_is_enabled(), which I would rather expect in fdt_support by name fdt_node_is_available(), or something like that. Also fdtdec does a strange thing with compatible strings: it declares an enum for compatible strings and then a map which maps this enum values to compatible strings... Why not just use the compatible strings? What is the purpose of having two files implementing fdt stuff? Marek ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: difference between fdtdec and fdt_support ? 2022-01-06 12:21 difference between fdtdec and fdt_support ? Marek Behún @ 2022-01-06 14:37 ` Rasmus Villemoes 2022-01-06 15:48 ` Simon Glass 1 sibling, 0 replies; 7+ messages in thread From: Rasmus Villemoes @ 2022-01-06 14:37 UTC (permalink / raw) To: Marek Behún, Simon Glass; +Cc: U-Boot Mailing List On 06/01/2022 13.21, Marek Behún wrote: > Also fdtdec does a strange thing with compatible strings: it declares > an enum for compatible strings and then a map which maps this enum > values to compatible strings... Why not just use the compatible strings? > > What is the purpose of having two files implementing fdt stuff? AFAIK, the primary purpose is to ensure powerpc binaries are bloated with useless string literals.</sarcasm> https://lore.kernel.org/u-boot/50288d08-3ab8-ef3c-5118-f74e30297104@prevas.dk/ Rasmus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: difference between fdtdec and fdt_support ? 2022-01-06 12:21 difference between fdtdec and fdt_support ? Marek Behún 2022-01-06 14:37 ` Rasmus Villemoes @ 2022-01-06 15:48 ` Simon Glass 2022-01-06 16:10 ` Marek Behún 1 sibling, 1 reply; 7+ messages in thread From: Simon Glass @ 2022-01-06 15:48 UTC (permalink / raw) To: Marek Behún; +Cc: U-Boot Mailing List Hi Marek, On Thu, 6 Jan 2022 at 05:21, Marek Behún <marek.behun@nic.cz> wrote: > > Hi Simon, > > I am a little confused. > > We have > common/fdt_support.c > and > lib/fdtdec.c > > The second one implements for example fdtdec_get_is_enabled(), which I > would rather expect in fdt_support by name fdt_node_is_available(), or > something like that. Should be moved to ofnode > > Also fdtdec does a strange thing with compatible strings: it declares > an enum for compatible strings and then a map which maps this enum > values to compatible strings... Why not just use the compatible strings? Did you see the comment? * NOTE: This list is basically a TODO list for things that need to be * converted to driver model. So don't add new things here unless there is a * good reason why driver-model conversion is infeasible. Examples include * things which are used before driver model is available. This is effectively a list of things that should be converted to driver model. The list should then go away. > > What is the purpose of having two files implementing fdt stuff? fdtdec - for reading from the DT. Should go away and be replaced with the ofnode API, and fdtaddr.c fdt_support - for updating the DT, e.g. for fixups before booting an OS Regards, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: difference between fdtdec and fdt_support ? 2022-01-06 15:48 ` Simon Glass @ 2022-01-06 16:10 ` Marek Behún 2022-01-06 16:15 ` Simon Glass 0 siblings, 1 reply; 7+ messages in thread From: Marek Behún @ 2022-01-06 16:10 UTC (permalink / raw) To: Simon Glass; +Cc: U-Boot Mailing List On Thu, 6 Jan 2022 08:48:48 -0700 Simon Glass <sjg@chromium.org> wrote: > Hi Marek, > > On Thu, 6 Jan 2022 at 05:21, Marek Behún <marek.behun@nic.cz> wrote: > > > > Hi Simon, > > > > I am a little confused. > > > > We have > > common/fdt_support.c > > and > > lib/fdtdec.c > > > > The second one implements for example fdtdec_get_is_enabled(), which I > > would rather expect in fdt_support by name fdt_node_is_available(), or > > something like that. > > Should be moved to ofnode ?? But this function is needed for example when fixing device tree for Linux. > > > > Also fdtdec does a strange thing with compatible strings: it declares > > an enum for compatible strings and then a map which maps this enum > > values to compatible strings... Why not just use the compatible strings? > > Did you see the comment? > > * NOTE: This list is basically a TODO list for things that need to be > * converted to driver model. So don't add new things here unless there is a > * good reason why driver-model conversion is infeasible. Examples include > * things which are used before driver model is available. > > This is effectively a list of things that should be converted to > driver model. The list should then go away. Hmm. But can't that be simply made into a list in a comment? Because currently this is compiled in for every board that uses any such function from fdtdec, even if they don't use any string from those compatible strings. > > > > What is the purpose of having two files implementing fdt stuff? > > fdtdec - for reading from the DT. Should go away and be replaced with > the ofnode API, and fdtaddr.c > fdt_support - for updating the DT, e.g. for fixups before booting an OS Thanks! Okay that makes sense. This means that we should also have fdt_node_is_available() in fdt_support.c which does the same thing. (For now this can be made a static inline function that calls fdtdec_get_is_enabled().) Marek ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: difference between fdtdec and fdt_support ? 2022-01-06 16:10 ` Marek Behún @ 2022-01-06 16:15 ` Simon Glass 2022-01-06 16:55 ` Marek Behún 0 siblings, 1 reply; 7+ messages in thread From: Simon Glass @ 2022-01-06 16:15 UTC (permalink / raw) To: Marek Behún; +Cc: U-Boot Mailing List Hi Marek, On Thu, 6 Jan 2022 at 09:10, Marek Behún <marek.behun@nic.cz> wrote: > > On Thu, 6 Jan 2022 08:48:48 -0700 > Simon Glass <sjg@chromium.org> wrote: > > > Hi Marek, > > > > On Thu, 6 Jan 2022 at 05:21, Marek Behún <marek.behun@nic.cz> wrote: > > > > > > Hi Simon, > > > > > > I am a little confused. > > > > > > We have > > > common/fdt_support.c > > > and > > > lib/fdtdec.c > > > > > > The second one implements for example fdtdec_get_is_enabled(), which I > > > would rather expect in fdt_support by name fdt_node_is_available(), or > > > something like that. > > > > Should be moved to ofnode > > ?? But this function is needed for example when fixing device tree for > Linux. > > > > > > > Also fdtdec does a strange thing with compatible strings: it declares > > > an enum for compatible strings and then a map which maps this enum > > > values to compatible strings... Why not just use the compatible strings? > > > > Did you see the comment? > > > > * NOTE: This list is basically a TODO list for things that need to be > > * converted to driver model. So don't add new things here unless there is a > > * good reason why driver-model conversion is infeasible. Examples include > > * things which are used before driver model is available. > > > > This is effectively a list of things that should be converted to > > driver model. The list should then go away. > > Hmm. But can't that be simply made into a list in a comment? Because > currently this is compiled in for every board that uses any such > function from fdtdec, even if they don't use any string from those > compatible strings. Well another option would be to delete the boards that need it, since no one has seen fit to resolve this all these years. Or create some drivers for them. > > > > > > > What is the purpose of having two files implementing fdt stuff? > > > > fdtdec - for reading from the DT. Should go away and be replaced with > > the ofnode API, and fdtaddr.c > > fdt_support - for updating the DT, e.g. for fixups before booting an OS > > Thanks! Okay that makes sense. This means that we should also have > fdt_node_is_available() in fdt_support.c which does the same thing. > (For now this can be made a static inline function that calls > fdtdec_get_is_enabled().) OK. BTW fdt_support.c should really move to use ofnode. There are few ofnode 'write' functions at present, but we should fill those out so we don't need to use flattree for anything, with OF_LIVE is enabled. Regards, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: difference between fdtdec and fdt_support ? 2022-01-06 16:15 ` Simon Glass @ 2022-01-06 16:55 ` Marek Behún 2022-01-27 15:05 ` Simon Glass 0 siblings, 1 reply; 7+ messages in thread From: Marek Behún @ 2022-01-06 16:55 UTC (permalink / raw) To: Simon Glass; +Cc: U-Boot Mailing List On Thu, 6 Jan 2022 09:15:17 -0700 Simon Glass <sjg@chromium.org> wrote: > Hi Marek, > > On Thu, 6 Jan 2022 at 09:10, Marek Behún <marek.behun@nic.cz> wrote: > > > > On Thu, 6 Jan 2022 08:48:48 -0700 > > Simon Glass <sjg@chromium.org> wrote: > > > > > Hi Marek, > > > > > > On Thu, 6 Jan 2022 at 05:21, Marek Behún <marek.behun@nic.cz> wrote: > > > > > > > > Hi Simon, > > > > > > > > I am a little confused. > > > > > > > > We have > > > > common/fdt_support.c > > > > and > > > > lib/fdtdec.c > > > > > > > > The second one implements for example fdtdec_get_is_enabled(), which I > > > > would rather expect in fdt_support by name fdt_node_is_available(), or > > > > something like that. > > > > > > Should be moved to ofnode > > > > ?? But this function is needed for example when fixing device tree for > > Linux. > > > > > > > > > > Also fdtdec does a strange thing with compatible strings: it declares > > > > an enum for compatible strings and then a map which maps this enum > > > > values to compatible strings... Why not just use the compatible strings? > > > > > > Did you see the comment? > > > > > > * NOTE: This list is basically a TODO list for things that need to be > > > * converted to driver model. So don't add new things here unless there is a > > > * good reason why driver-model conversion is infeasible. Examples include > > > * things which are used before driver model is available. > > > > > > This is effectively a list of things that should be converted to > > > driver model. The list should then go away. > > > > Hmm. But can't that be simply made into a list in a comment? Because > > currently this is compiled in for every board that uses any such > > function from fdtdec, even if they don't use any string from those > > compatible strings. > > Well another option would be to delete the boards that need it, since > no one has seen fit to resolve this all these years. Or create some > drivers for them. I still don't quite understand why the compatible strings have to be in this fdtdec.c file. For example in arch/arm/mach-socfpga/clock_manager_arria10.c we call node = fdtdec_next_compatible(blob, 0, COMPAT_ALTERA_SOCFPGA_CLK_INIT); This constant, COMPAT_ALTERA_SOCFPGA_CLK_INIT, is an enum constant, which is then in fdtdec.c translated to string compatible with compat_names[id] for which fdt_node_offset_by_compatible() is used. So why not simply put this string constant into arch/arm/mach-socfpga/clock_manager_arria10.c by calling node = fdt_node_offset_by_compatible(blob, node, "altr,socfpga-a10-clk-init"); ?? That way at least the string literals won't be compiled in into every u-boot binary, even those that don't need those literals at all. > > > > > > > > > > What is the purpose of having two files implementing fdt stuff? > > > > > > fdtdec - for reading from the DT. Should go away and be replaced with > > > the ofnode API, and fdtaddr.c > > > fdt_support - for updating the DT, e.g. for fixups before booting an OS > > > > Thanks! Okay that makes sense. This means that we should also have > > fdt_node_is_available() in fdt_support.c which does the same thing. > > (For now this can be made a static inline function that calls > > fdtdec_get_is_enabled().) > > OK. > > BTW fdt_support.c should really move to use ofnode. There are few > ofnode 'write' functions at present, but we should fill those out so > we don't need to use flattree for anything, with OF_LIVE is enabled. Can OF_LIVE then generate dtb for kernel? Marek ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: difference between fdtdec and fdt_support ? 2022-01-06 16:55 ` Marek Behún @ 2022-01-27 15:05 ` Simon Glass 0 siblings, 0 replies; 7+ messages in thread From: Simon Glass @ 2022-01-27 15:05 UTC (permalink / raw) To: Marek Behún; +Cc: U-Boot Mailing List Hi Marek, On Thu, 6 Jan 2022 at 09:55, Marek Behún <marek.behun@nic.cz> wrote: > > On Thu, 6 Jan 2022 09:15:17 -0700 > Simon Glass <sjg@chromium.org> wrote: > > > Hi Marek, > > > > On Thu, 6 Jan 2022 at 09:10, Marek Behún <marek.behun@nic.cz> wrote: > > > > > > On Thu, 6 Jan 2022 08:48:48 -0700 > > > Simon Glass <sjg@chromium.org> wrote: > > > > > > > Hi Marek, > > > > > > > > On Thu, 6 Jan 2022 at 05:21, Marek Behún <marek.behun@nic.cz> wrote: > > > > > > > > > > Hi Simon, > > > > > > > > > > I am a little confused. > > > > > > > > > > We have > > > > > common/fdt_support.c > > > > > and > > > > > lib/fdtdec.c > > > > > > > > > > The second one implements for example fdtdec_get_is_enabled(), which I > > > > > would rather expect in fdt_support by name fdt_node_is_available(), or > > > > > something like that. > > > > > > > > Should be moved to ofnode > > > > > > ?? But this function is needed for example when fixing device tree for > > > Linux. > > > > > > > > > > > > > Also fdtdec does a strange thing with compatible strings: it declares > > > > > an enum for compatible strings and then a map which maps this enum > > > > > values to compatible strings... Why not just use the compatible strings? > > > > > > > > Did you see the comment? > > > > > > > > * NOTE: This list is basically a TODO list for things that need to be > > > > * converted to driver model. So don't add new things here unless there is a > > > > * good reason why driver-model conversion is infeasible. Examples include > > > > * things which are used before driver model is available. > > > > > > > > This is effectively a list of things that should be converted to > > > > driver model. The list should then go away. > > > > > > Hmm. But can't that be simply made into a list in a comment? Because > > > currently this is compiled in for every board that uses any such > > > function from fdtdec, even if they don't use any string from those > > > compatible strings. > > > > Well another option would be to delete the boards that need it, since > > no one has seen fit to resolve this all these years. Or create some > > drivers for them. > > I still don't quite understand why the compatible strings have to be in > this fdtdec.c file. > > For example in > arch/arm/mach-socfpga/clock_manager_arria10.c > we call > node = fdtdec_next_compatible(blob, 0, > COMPAT_ALTERA_SOCFPGA_CLK_INIT); > This constant, COMPAT_ALTERA_SOCFPGA_CLK_INIT, is an enum constant, > which is then in fdtdec.c translated to string compatible with > compat_names[id] > for which fdt_node_offset_by_compatible() is used. > > So why not simply put this string constant into > arch/arm/mach-socfpga/clock_manager_arria10.c > by calling > node = fdt_node_offset_by_compatible(blob, node, > "altr,socfpga-a10-clk-init"); > ?? > > That way at least the string literals won't be compiled in into every > u-boot binary, even those that don't need those literals at all. Yes, indeed, but really these uses should be dropped and converted to driver model. > > > > > > > > > > > > > > What is the purpose of having two files implementing fdt stuff? > > > > > > > > fdtdec - for reading from the DT. Should go away and be replaced with > > > > the ofnode API, and fdtaddr.c > > > > fdt_support - for updating the DT, e.g. for fixups before booting an OS > > > > > > Thanks! Okay that makes sense. This means that we should also have > > > fdt_node_is_available() in fdt_support.c which does the same thing. > > > (For now this can be made a static inline function that calls > > > fdtdec_get_is_enabled().) > > > > OK. > > > > BTW fdt_support.c should really move to use ofnode. There are few > > ofnode 'write' functions at present, but we should fill those out so > > we don't need to use flattree for anything, with OF_LIVE is enabled. > > Can OF_LIVE then generate dtb for kernel? It isn't implemented, but we should add it. It is a simple piece of code to flatten the tree. Regards, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-27 15:07 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-06 12:21 difference between fdtdec and fdt_support ? Marek Behún 2022-01-06 14:37 ` Rasmus Villemoes 2022-01-06 15:48 ` Simon Glass 2022-01-06 16:10 ` Marek Behún 2022-01-06 16:15 ` Simon Glass 2022-01-06 16:55 ` Marek Behún 2022-01-27 15:05 ` Simon Glass
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.