Re: [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
Hi Simon, On 11/24/21 21:12, Simon Glass wrote: Hi Walter, On Mon, 2 Aug 2021 at 13:29, Walter Lozano wrote: Hi Simon, On 8/1/21 11:50 PM, Simon Glass wrote: Hi Walter, On Sun, 1 Aug 2021 at 20:45, Walter Lozano wrote: Hi Simon, Thanks for checking this bug, I'm glad that you were able to come with fix quickly. I have some questions, I hope that you find some time to help me understand. On 7/28/21 10:23 PM, Simon Glass wrote: The current name is confusing because the logic is actually backwards from what you might expect. Rename it to needs_widening() and update the comments. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 3996971e39c..9749966d5fb 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -24,16 +24,19 @@ from patman import tools # A list of types we support class Type(IntEnum): +# Types in order from widest to narrowest (BYTE, INT, STRING, BOOL, INT64) = range(5) Sorry but I don't understand why BYTE is wider than INT (or INT64) I think perhaps we need a better name. A wider type is one that can hold the values of a narrower one, plus more. In this case a 'bytes' type can hold anything (bytes, int, int64, bool) so is the 'widest' there is. It is the lowest common denominator in the devicetree. Thanks for taking the time to explain. I understand the idea behind your explanation but I still not sure if I follow you completely. In any case, let me add a few words in order to be more clear. It is my impression that when you say 'bytes' (and not BYTE like in the declaration) you are referring to a list. Is that the case? If not, BYTE (8 bit) seems to be narrower than INT (32 bits), isn't it? A bit long in finding this email again... Actually bytes means the Python bytes type which holds a sequence of bytes. So it can hold an int, whereas an int cannot hold a bytes value, in general (although it could if it happened to be 4 bytes). It is more clear now, thank you. Also, another example is INT, BOOL and INT64. It is clear that INT is wider than BOOL, but why BOOL is wider than INT64? It isn't actually, but INT64 is a bit of a special case and I had to put it somewhere. I see, thank you for the explanation! As reference I have been checking https://devicetree-specification.readthedocs.io/en/stable/devicetree-basics.html#property-values [..] Regards, Walter -- Walter Lozano Collabora Ltd.
Re: [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
Hi Simon, On 8/1/21 11:50 PM, Simon Glass wrote: Hi Walter, On Sun, 1 Aug 2021 at 20:45, Walter Lozano wrote: Hi Simon, Thanks for checking this bug, I'm glad that you were able to come with fix quickly. I have some questions, I hope that you find some time to help me understand. On 7/28/21 10:23 PM, Simon Glass wrote: The current name is confusing because the logic is actually backwards from what you might expect. Rename it to needs_widening() and update the comments. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 3996971e39c..9749966d5fb 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -24,16 +24,19 @@ from patman import tools # A list of types we support class Type(IntEnum): +# Types in order from widest to narrowest (BYTE, INT, STRING, BOOL, INT64) = range(5) Sorry but I don't understand why BYTE is wider than INT (or INT64) I think perhaps we need a better name. A wider type is one that can hold the values of a narrower one, plus more. In this case a 'bytes' type can hold anything (bytes, int, int64, bool) so is the 'widest' there is. It is the lowest common denominator in the devicetree. Thanks for taking the time to explain. I understand the idea behind your explanation but I still not sure if I follow you completely. In any case, let me add a few words in order to be more clear. It is my impression that when you say 'bytes' (and not BYTE like in the declaration) you are referring to a list. Is that the case? If not, BYTE (8 bit) seems to be narrower than INT (32 bits), isn't it? Also, another example is INT, BOOL and INT64. It is clear that INT is wider than BOOL, but why BOOL is wider than INT64? As reference I have been checking https://devicetree-specification.readthedocs.io/en/stable/devicetree-basics.html#property-values -def is_wider_than(self, other): -"""Check if another type is 'wider' than this one +def needs_widening(self, other): +"""Check if this type needs widening to hold a value from another type -A wider type is one that holds more information than an earlier one, -similar to the concept of type-widening in C. +A wider type is one that can hold a wider array of information than +another one, or is less restrictive, so it can hold the information of +another type as well as its own. This is similar to the concept of +type-widening in C. This uses a simple arithmetic comparison, since type values are in order -from narrowest (BYTE) to widest (INT64). +from widest (BYTE) to narrowest (INT64). Args: other: Other type to compare against @@ -149,7 +152,7 @@ class Prop: update the current property to be like the second, since it is less specific. """ -if self.type.is_wider_than(newprop.type): +if self.type.needs_widening(newprop.type): if self.type == Type.INT and newprop.type == Type.BYTE: if type(self.value) == list: new_value = [] Regards, Simon Regards, Walter
Re: [PATCH] dtoc: Correct the intarray-widening test case
Hi Simon, On 8/2/21 10:37 AM, Simon Glass wrote: This case was intended to check that widening an int array with an int does nothing. Fix it. Reported-by: Walter Lozano Signed-off-by: Simon Glass --- tools/dtoc/test_fdt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 1119e6b7847..d104f3c7745 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -425,7 +425,7 @@ class TestProp(unittest.TestCase): # Widen an array of ints with an int (should do nothing) prop = self.node.props['intarray'] -prop2 = node2.props['intarray'] +prop2 = node2.props['intval'] self.assertEqual(Type.INT, prop.type) self.assertEqual(3, len(prop.value)) prop.Widen(prop2) Thanks for the fix! Reviewed-by: Walter Lozano Regards, Walter
Re: [PATCH 2/3] dtoc: Fix widening an int array to an int
Hi Simon, I know you already merged this one but I have some doubts I would like to check with you. On 7/28/21 10:23 PM, Simon Glass wrote: An int array can hold a single int so we should not need to do anything in the widening operation. However due to a quirk in the code, an int[3] widened with an int produced an int[4]. Fix this and add a test. Fix a comment typo while we are here. Signed-off-by: Simon Glass Reported-by: Tom Rini --- test/dm/of_platdata.c | 4 +--- tools/dtoc/fdt.py | 15 --- tools/dtoc/test_dtoc.py | 6 +++--- tools/dtoc/test_fdt.py | 11 ++- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c index 0f89c7a7da8..e3fa01afddf 100644 --- a/test/dm/of_platdata.c +++ b/test/dm/of_platdata.c @@ -35,11 +35,10 @@ static int dm_test_of_plat_props(struct unit_test_state *uts) plat = dev_get_plat(dev); ut_assert(plat->boolval); ut_asserteq(1, plat->intval); - ut_asserteq(4, ARRAY_SIZE(plat->intarray)); + ut_asserteq(3, ARRAY_SIZE(plat->intarray)); ut_asserteq(2, plat->intarray[0]); ut_asserteq(3, plat->intarray[1]); ut_asserteq(4, plat->intarray[2]); - ut_asserteq(0, plat->intarray[3]); ut_asserteq(5, plat->byteval); ut_asserteq(3, ARRAY_SIZE(plat->bytearray)); ut_asserteq(6, plat->bytearray[0]); @@ -61,7 +60,6 @@ static int dm_test_of_plat_props(struct unit_test_state *uts) ut_asserteq(5, plat->intarray[0]); ut_asserteq(0, plat->intarray[1]); ut_asserteq(0, plat->intarray[2]); - ut_asserteq(0, plat->intarray[3]); ut_asserteq(8, plat->byteval); ut_asserteq(3, ARRAY_SIZE(plat->bytearray)); ut_asserteq(1, plat->bytearray[0]); diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 9749966d5fb..429e95f9a96 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -163,13 +163,14 @@ class Prop: self.value = new_value self.type = newprop.type -if type(newprop.value) == list and type(self.value) != list: -self.value = [self.value] - -if type(self.value) == list and len(newprop.value) > len(self.value): -val = self.GetEmpty(self.type) -while len(self.value) < len(newprop.value): -self.value.append(val) +if type(newprop.value) == list: +if type(self.value) != list: +self.value = [self.value] + +if len(newprop.value) > len(self.value): +val = self.GetEmpty(self.type) +while len(self.value) < len(newprop.value): +self.value.append(val) @classmethod def GetEmpty(self, type): diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 863ede90b7a..44d5d0c354a 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -296,7 +296,7 @@ struct dtd_sandbox_spl_test { \tbool\t\tboolval; \tunsigned char\tbytearray[3]; \tunsigned char\tbyteval; -\tfdt32_t\t\tintarray[4]; +\tfdt32_t\t\tintarray[3]; \tfdt32_t\t\tintval; \tunsigned char\tlongbytearray[9]; \tunsigned char\tnotstring[5]; @@ -354,7 +354,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test = { \t.boolval\t\t= true, \t.bytearray\t\t= {0x6, 0x0, 0x0}, \t.byteval\t\t= 0x5, -\t.intarray\t\t= {0x2, 0x3, 0x4, 0x0}, +\t.intarray\t\t= {0x2, 0x3, 0x4}, \t.intval\t\t\t= 0x1, \t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10, \t\t0x11}, @@ -377,7 +377,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test2 = { \t.acpi_name\t\t= "_SB.GPO0", \t.bytearray\t\t= {0x1, 0x23, 0x34}, \t.byteval\t\t= 0x8, -\t.intarray\t\t= {0x5, 0x0, 0x0, 0x0}, +\t.intarray\t\t= {0x5, 0x0, 0x0}, \t.intval\t\t\t= 0x3, \t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0x0, 0x0, 0x0, 0x0, \t\t0x0}, diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 856392b1bd9..857861c14ed 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -379,7 +379,7 @@ class TestProp(unittest.TestCase): self.assertEqual(Type.INT, prop.type) self.assertEqual(1, fdt32_to_cpu(prop.value)) -# Convert singla value to array +# Convert single value to array prop2 = self.node.props['intarray'] prop.Widen(prop2) self.assertEqual(Type.INT, prop.type) @@ -422,6 +422,15 @@ class TestProp(unittest.TestCase): self.assertTrue(isinstance(prop.value, list)) self.assertEqual(3, len(prop.value)) +# Widen an array of ints with an int (should do nothing) +prop = self.node.props['intarray'] +prop2 = node2.props['intarray'] I was expecting intval instead of intarray here. +self.assertEqual(Type.INT, prop.type) +self.assertEqual(3, len(prop.value)) +prop.Widen(prop2) +self.assertEqual(Type.INT, prop.type) +self.assertEqual(3,
Re: [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
Hi Simon, Thanks for checking this bug, I'm glad that you were able to come with fix quickly. I have some questions, I hope that you find some time to help me understand. On 7/28/21 10:23 PM, Simon Glass wrote: The current name is confusing because the logic is actually backwards from what you might expect. Rename it to needs_widening() and update the comments. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 3996971e39c..9749966d5fb 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -24,16 +24,19 @@ from patman import tools # A list of types we support class Type(IntEnum): +# Types in order from widest to narrowest (BYTE, INT, STRING, BOOL, INT64) = range(5) Sorry but I don't understand why BYTE is wider than INT (or INT64) -def is_wider_than(self, other): -"""Check if another type is 'wider' than this one +def needs_widening(self, other): +"""Check if this type needs widening to hold a value from another type -A wider type is one that holds more information than an earlier one, -similar to the concept of type-widening in C. +A wider type is one that can hold a wider array of information than +another one, or is less restrictive, so it can hold the information of +another type as well as its own. This is similar to the concept of +type-widening in C. This uses a simple arithmetic comparison, since type values are in order -from narrowest (BYTE) to widest (INT64). +from widest (BYTE) to narrowest (INT64). Args: other: Other type to compare against @@ -149,7 +152,7 @@ class Prop: update the current property to be like the second, since it is less specific. """ -if self.type.is_wider_than(newprop.type): +if self.type.needs_widening(newprop.type): if self.type == Type.INT and newprop.type == Type.BYTE: if type(self.value) == list: new_value = [] Regards, Walter
Re: [PATCH] dtoc: Check that a parent is not missing
Hi Simon, Thanks for the patch, it will be a nice way to spot errors and avoid headaches! On 7/7/21 8:22 AM, Simon Glass wrote: With of-platdata-inst we want to set up a reference to each devices' parent device, if there is one. If we find that the device has a parent (i.e. is not a root node) but it is not in the list of devices being written, then we cannot create the reference. Report an error in this case, since it indicates that the parent node is either missing a compatible string, is disabled, or perhaps does not have any properties because it was not tagged for SPL. Signed-off-by: Simon Glass --- tools/dtoc/dtb_platdata.py | 9 tools/dtoc/test/dtoc_test_noparent.dts | 32 ++ tools/dtoc/test_dtoc.py| 10 3 files changed, 51 insertions(+) create mode 100644 tools/dtoc/test/dtoc_test_noparent.dts diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 2d42480a9a5..a951a5a2264 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -749,6 +749,15 @@ class DtbPlatdata(): break if node.parent and node.parent.parent: +if node.parent not in self._valid_nodes: +# This might indicate that the parent node is not in the +# SPL/TPL devicetree but the child is. For example if we are +# dealing with of-platdata in TPL, the parent has a +# u-boot,dm-spl tag but the child has u-boot,dm-pre-reloc. In +# this case the child node exists in TPL but the parent does +# not. +raise ValueError("Node '%s' requires parent node '%s' but it is not in the valid list" % + (node.path, node.parent.path)) self.buf('\t.parent\t\t= DM_DEVICE_REF(%s),\n' % node.parent.var_name) if priv_name: diff --git a/tools/dtoc/test/dtoc_test_noparent.dts b/tools/dtoc/test/dtoc_test_noparent.dts new file mode 100644 index 000..5a820301e72 --- /dev/null +++ b/tools/dtoc/test/dtoc_test_noparent.dts @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2017 Google, Inc + */ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + i2c@0 { + compatible = "sandbox,i2c"; + u-boot,dm-pre-tpl; Does u-boot,dm-pre-tpl exists? Probably I'm missing something here but I was expecting u-boot,dm-spl. + #address-cells = <1>; + #size-cells = <0>; + spl-test { + u-boot,dm-pre-reloc; + compatible = "sandbox,spl-test"; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + pmic@9 { + compatible = "sandbox,pmic"; + u-boot,dm-pre-reloc; + reg = <9>; + low-power; + }; + }; + }; +}; diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 0b2805feed2..863ede90b7a 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -1830,3 +1830,13 @@ U_BOOT_DRVINFO(spl_test2) = { dtb_file = get_dtb_file('dtoc_test_single_reg.dts') output = tools.GetOutputFilename('output') self.run_test(['struct'], dtb_file, output) + +def test_missing_parent(self): +"""Test detection of a parent node with no properties""" +dtb_file = get_dtb_file('dtoc_test_noparent.dts', capture_stderr=True) +output = tools.GetOutputFilename('output') +with self.assertRaises(ValueError) as exc: +self.run_test(['device'], dtb_file, output, instantiate=True) +self.assertIn("Node '/i2c@0/spl-test/pmic@9' requires parent node " + "'/i2c@0/spl-test' but it is not in the valid list", + str(exc.exception)) Regards, Walter
Re: [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail
Hi Simon, On 7/4/21 3:19 PM, Simon Glass wrote: When things go wrong it can be confusing to figure out what to change. Add a few more details to the documentation. Fix a 'make htmldocs' warning while we are here. Signed-off-by: Simon Glass --- doc/develop/driver-model/of-plat.rst | 53 1 file changed, 53 insertions(+) diff --git a/doc/develop/driver-model/of-plat.rst b/doc/develop/driver-model/of-plat.rst index 74f1932473b..e2763965839 100644 --- a/doc/develop/driver-model/of-plat.rst +++ b/doc/develop/driver-model/of-plat.rst @@ -597,6 +597,59 @@ as a macro included in the driver definition:: +Problems + + +In some cases you will you see something like this:: + + WARNING: the driver rockchip_rk3188_grf was not found in the driver list + +The driver list is a list of drivers, each with a name. The name is in the +U_BOOT_DRIVER() declaration, repeated twice, one in brackets and once as the +.name member. For example, in the following declaration the driver name is +`rockchip_rk3188_grf`:: + + U_BOOT_DRIVER(rockchip_rk3188_grf) = { + .name = "rockchip_rk3188_grf", + .id = UCLASS_SYSCON, + .of_match = rk3188_syscon_ids + 1, + .bind = rk3188_syscon_bind_of_plat, + }; + +The first name U_BOOT_DRIVER(xx) is used to create a linker symbol so that the +driver can be accessed at build-time without any overhead. The second one +(.name = "xx") is used at runtime when something wants to print out the driver +name. + +The dtoc tool expects to be able to find a driver for each compatible string in +the devicetree. For example, if the devicetree has:: + + grf: grf@20008000 { + compatible = "rockchip,rk3188-grf", "syscon"; + reg = <0x20008000 0x200>; + u-boot,dm-spl; + }; + +then dtoc looks at the first compatible string ("rockchip,rk3188-grf"), +converts that to a C identifier (rockchip_rk3188_grf) and then looks for that. + +Various things can cause dtoc to fail to find the driver and it tries to +warn about these. For example: + + rockchip_rk3188_uart: Missing .compatible in drivers/serial/serial_rockchip.c +: WARNING: the driver rockchip_rk3188_uart was not found in the driver list + +Without a compatible string a driver cannot be used by dtoc, even if the +compatible string is not actually needed at runtime. + +If the problem is simply that there are multiple compatible strings, the +DM_DRIVER_ALIAS() macro can be used to tell dtoc about this and avoid a problem. + +Checks are also made the referenced driver has a .compatible member and a .id +member. The first provides the array of compatible strings and the second +provides the uclass ID. + This paragraph sound strange, maybe "Checks are are also made to confirm that the referenced driver..." but your English is better than mine. Reviewed-by: Walter Lozano Regards, Walter + Caveats ---
Re: [PATCH 7/8] dtoc: Detect drivers which do not parse correctly
On 7/4/21 3:19 PM, Simon Glass wrote: At present if a driver is missing a uclass or compatible stirng, this Most probably it should be "string" is silently ignored. This makes sense in most cases, particularly for the compatible string, since it is not required except when the driver is used with of-platdata. But it is also not very helpful. When there is some sort of problem with a driver, the missing compatible string (for example) may be the cause. Add a warning in this case, showing it only for drivers which are used by the build. Signed-off-by: Simon Glass --- tools/dtoc/src_scan.py | 7 ++- tools/dtoc/test_src_scan.py | 38 + 2 files changed, 44 insertions(+), 1 deletion(-) Reviewed-by: Walter Lozano Thanks! Walter diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py index 847677757d9..3bef59d616e 100644 --- a/tools/dtoc/src_scan.py +++ b/tools/dtoc/src_scan.py @@ -546,7 +546,12 @@ class Scanner: # The driver does not have a uclass or compat string. # The first is required but the second is not, so just # ignore this. -pass +if not driver.uclass_id: +warn = 'Missing .uclass' +else: +warn = 'Missing .compatible' +self._warnings[driver.name].add('%s in %s' % +(warn, fname)) driver = None ids_name = None compat = None diff --git a/tools/dtoc/test_src_scan.py b/tools/dtoc/test_src_scan.py index 62500e80fe7..f03cf8ed7c7 100644 --- a/tools/dtoc/test_src_scan.py +++ b/tools/dtoc/test_src_scan.py @@ -581,3 +581,41 @@ rockchip_rk3288_grf: WARNING: the driver rockchip_rk3288_grf was not found in th ''', stdout.getvalue()) self.assertIn('tegra_i2c_ids', stdout.getvalue()) + +def scan_uclass_warning(self): +"""Test a missing .uclass in the driver""" +buff = ''' +static const struct udevice_id tegra_i2c_ids[] = { + { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 }, + { } +}; + +U_BOOT_DRIVER(i2c_tegra) = { + .name = "i2c_tegra", + .of_match = tegra_i2c_ids, +}; +''' +scan = src_scan.Scanner(None, None) +scan._parse_driver('file.c', buff) +self.assertEqual( +{'i2c_tegra': {'Missing .uclass in file.c'}}, +scan._warnings) + +def scan_compat_warning(self): +"""Test a missing .compatible in the driver""" +buff = ''' +static const struct udevice_id tegra_i2c_ids[] = { + { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 }, + { } +}; + +U_BOOT_DRIVER(i2c_tegra) = { + .name = "i2c_tegra", + .id = UCLASS_I2C, +}; +''' +scan = src_scan.Scanner(None, None) +scan._parse_driver('file.c', buff) +self.assertEqual( +{'i2c_tegra': {'Missing .compatible in file.c'}}, +scan._warnings)
Re: [PATCH 4/8] dtoc: Correct the re_compat regular expression
Hi Simon, On 7/4/21 3:19 PM, Simon Glass wrote: This expects a . before the field name (.e.g '.compatible = ...) but presently accepts anything at all. Fix it. Signed-off-by: Simon Glass --- tools/dtoc/src_scan.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Walter Lozano Thanks! Walter diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py index 6c37a71e978..6e8e1ba51a0 100644 --- a/tools/dtoc/src_scan.py +++ b/tools/dtoc/src_scan.py @@ -452,8 +452,8 @@ class Scanner: # Collect the compatible string, e.g. 'rockchip,rk3288-grf' compat = None -re_compat = re.compile(r'{\s*.compatible\s*=\s*"(.*)"\s*' - r'(,\s*.data\s*=\s*(\S*))?\s*},') +re_compat = re.compile(r'{\s*\.compatible\s*=\s*"(.*)"\s*' + r'(,\s*\.data\s*=\s*(\S*))?\s*},') # This is a dict of compatible strings that were found: #key: Compatible string, e.g. 'rockchip,rk3288-grf'
Re: [PATCH 3/8] dtoc: Allow multiple warnings for a driver
Hi Simon, On 7/4/21 3:19 PM, Simon Glass wrote: At present we show when a driver is missing but this is not always that useful. There are various reasons way a driver may appear to be missing, Did you mean "why" instead of "way"? such as a parse error in the source code or a missing field in the driver declaration. Update the implementation to record all warnings for each driver, showing only those which relate to drivers that are actually used. This avoids spamming the user with warnings related to a driver for a different board. Signed-off-by: Simon Glass --- tools/dtoc/src_scan.py | 24 1 file changed, 20 insertions(+), 4 deletions(-) Reviewed-by: Walter Lozano Thank you, it is indeed something worth to be added! Walter diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py index 1dbb56712a3..6c37a71e978 100644 --- a/tools/dtoc/src_scan.py +++ b/tools/dtoc/src_scan.py @@ -13,6 +13,7 @@ U_BOOT_DRIVER(), UCLASS_DRIVER and all struct declarations in header files. See doc/driver-model/of-plat.rst for more informaiton """ +import collections import os import re import sys @@ -190,6 +191,9 @@ class Scanner: value: Driver name declared with U_BOOT_DRIVER(driver_name) _drivers_additional (list or str): List of additional drivers to use during scanning +_warnings: Dict of warnings found: +key: Driver name +value: Set of warnings _of_match: Dict holding information about compatible strings key: Name of struct udevice_id variable value: Dict of compatible info in that variable: @@ -217,6 +221,7 @@ class Scanner: self._driver_aliases = {} self._drivers_additional = drivers_additional or [] self._missing_drivers = set() +self._warnings = collections.defaultdict(set) self._of_match = {} self._compat_to_driver = {} self._uclass = {} @@ -267,7 +272,10 @@ class Scanner: aliases_c.remove(compat_c) return compat_c, aliases_c -self._missing_drivers.add(compat_list_c[0]) +name = compat_list_c[0] +self._missing_drivers.add(name) +self._warnings[name].add( +'WARNING: the driver %s was not found in the driver list' % name) return compat_list_c[0], compat_list_c[1:] @@ -577,9 +585,17 @@ class Scanner: def show_warnings(self): """Show any warnings that have been collected""" -for name in sorted(list(self._missing_drivers)): -print('WARNING: the driver %s was not found in the driver list' - % name) +used_drivers = [drv.name for drv in self._drivers.values() if drv.used] +missing = self._missing_drivers +for name in sorted(self._warnings.keys()): +if name in missing or name in used_drivers: +warns = sorted(list(self._warnings[name])) +# For now there is only ever one warning +print('%s: %s' % (name, warns[0])) +indent = ' ' * len(name) +if name in missing: +missing.remove(name) +print() def scan_driver(self, fname): """Scan a driver file to build a list of driver names and aliases
Re: [PATCH 2/8] dtoc: Convert to use ArgumentParser
Hi Simon, On 7/4/21 3:19 PM, Simon Glass wrote: Use this parser instead of OptionParser, which is deprecated. Signed-off-by: Simon Glass --- tools/dtoc/main.py | 51 -- 1 file changed, 27 insertions(+), 24 deletions(-) Reviewed-by: Walter Lozano Thanks! Walter diff --git a/tools/dtoc/main.py b/tools/dtoc/main.py index 93706de89bf..6f9b526bd74 100755 --- a/tools/dtoc/main.py +++ b/tools/dtoc/main.py @@ -21,7 +21,7 @@ options. For more information about the use of this options and tool please see doc/driver-model/of-plat.rst """ -from optparse import OptionParser +from argparse import ArgumentParser import os import sys import unittest @@ -51,7 +51,7 @@ def run_tests(processes, args): result = unittest.TestResult() sys.argv = [sys.argv[0]] -test_name = args and args[0] or None +test_name = args.files and args.files[0] or None test_dtoc.setup() @@ -66,47 +66,50 @@ def RunTestCoverage(): """Run the tests and check that we get 100% coverage""" sys.argv = [sys.argv[0]] test_util.RunTestCoverage('tools/dtoc/dtoc', '/main.py', -['tools/patman/*.py', '*/fdt*', '*test*'], options.build_dir) +['tools/patman/*.py', '*/fdt*', '*test*'], args.build_dir) if __name__ != '__main__': sys.exit(1) -parser = OptionParser() -parser.add_option('-B', '--build-dir', type='string', default='b', +epilog = '''Generate C code from devicetree files. See of-plat.rst for details''' + +parser = ArgumentParser(epilog=epilog) +parser.add_argument('-B', '--build-dir', type=str, default='b', help='Directory containing the build output') -parser.add_option('-c', '--c-output-dir', action='store', +parser.add_argument('-c', '--c-output-dir', action='store', help='Select output directory for C files') -parser.add_option('-C', '--h-output-dir', action='store', +parser.add_argument('-C', '--h-output-dir', action='store', help='Select output directory for H files (defaults to --c-output-di)') -parser.add_option('-d', '--dtb-file', action='store', +parser.add_argument('-d', '--dtb-file', action='store', help='Specify the .dtb input file') -parser.add_option('-i', '--instantiate', action='store_true', default=False, +parser.add_argument('-i', '--instantiate', action='store_true', default=False, help='Instantiate devices to avoid needing device_bind()') -parser.add_option('--include-disabled', action='store_true', +parser.add_argument('--include-disabled', action='store_true', help='Include disabled nodes') -parser.add_option('-o', '--output', action='store', +parser.add_argument('-o', '--output', action='store', help='Select output filename') -parser.add_option('-p', '--phase', type=str, +parser.add_argument('-p', '--phase', type=str, help='set phase of U-Boot this invocation is for (spl/tpl)') -parser.add_option('-P', '--processes', type=int, +parser.add_argument('-P', '--processes', type=int, help='set number of processes to use for running tests') -parser.add_option('-t', '--test', action='store_true', dest='test', +parser.add_argument('-t', '--test', action='store_true', dest='test', default=False, help='run tests') -parser.add_option('-T', '--test-coverage', action='store_true', -default=False, help='run tests and check for 100% coverage') -(options, args) = parser.parse_args() +parser.add_argument('-T', '--test-coverage', action='store_true', +default=False, help='run tests and check for 100%% coverage') +parser.add_argument('files', nargs='*') +args = parser.parse_args() # Run our meagre tests -if options.test: -ret_code = run_tests(options.processes, args) +if args.test: +ret_code = run_tests(args.processes, args) sys.exit(ret_code) -elif options.test_coverage: +elif args.test_coverage: RunTestCoverage() else: -dtb_platdata.run_steps(args, options.dtb_file, options.include_disabled, - options.output, - [options.c_output_dir, options.h_output_dir], - options.phase, instantiate=options.instantiate) +dtb_platdata.run_steps(args.files, args.dtb_file, args.include_disabled, + args.output, + [args.c_output_dir, args.h_output_dir], + args.phase, instantiate=args.instantiate)
Re: [PATCH 1/8] dtoc: Avoid using subscripts on match objects
Hi Simon, On 7/4/21 3:19 PM, Simon Glass wrote: These are not supported before Python 3.6 so avoid them. Signed-off-by: Simon Glass --- tools/dtoc/src_scan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Walter Lozano Thanks! Walter diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py index 2db96884c85..1dbb56712a3 100644 --- a/tools/dtoc/src_scan.py +++ b/tools/dtoc/src_scan.py @@ -555,7 +555,7 @@ class Scanner: if ids_m: ids_name = ids_m.group(1) elif m_alias: -self._driver_aliases[m_alias[2]] = m_alias[1] +self._driver_aliases[m_alias.group(2)] = m_alias.group(1) # Make the updates based on what we found for driver in drivers.values():
Re: [PATCH 00/30] dm: Implement OF_PLATDATA_INST in driver model (part E)
Hi Simon, On 12/31/20 1:09 AM, Simon Glass wrote: This series builds on the recent dtoc implementation of build-time device instantiation. It adds the required driver model support, which is basically a few more data structures. With this, sandbox_spl and chromebook_coral both use the new feature. For coral TPL there is a 1.5KB code-size reduction and a 1.75KB data-size increase: text data bss dec hex 18836 3080 12 2192855a8original 17229 4896 12 221375679with OF_PLATDATA_INST 17277 4896 12 2218556a9with OF_PLATDATA_RT The extra data size is due to the build-time devices which are now included in the image instead of being built at runtime. Also the private data for each device is allocated in the data region at present, even through much of it is just zeroes. The reduction in code size is due to not needing to bind devices at runtime, as a well as a simplified probe() function. Coral requires that data be copied out to RAM before being updated, so that adds a small amount to size (shown in the third line). Quite a lot of future work is possible, including reducing the size of data structures. See [1] for more ideas. But this series implements the basic feature. To try this out on your board, define CONFIG_SPL_OF_PLATDATA_INST and see what you get. Note: SPL tests do not yet pass with this series. The driver_rt struct is not set up so device_get_by_driver_info_idx() does not work. This means that looking up phandles will fail. This will be addressed in a v2 series, along with documentation updates and a little more information on code-size impact. This series is available at u-boot-dm/tin-working [1] https://lists.denx.de/pipermail/u-boot/2020-July/418433.html Thanks for this series. I have done a quick review, but I plan to do a deeper one and some testing during the following days. Regards, Walter Simon Glass (30): sandbox: Drop debug message in os_spl_to_uboot() linker_lists: Allow use in data structures dm: core: Add macros to access the new linker lists dm: core: Allow dropping run-time binding of devices dm: core: Adjust uclass setup with of-platdata dm: core: Set up driver model for OF_PLATDATA_INST dm: core: Skip adding uclasses with OF_PLATDATA_INST dm: Add the new dtoc-generated files to the build dm: core: Include dt-decl.h automatically dm: test: Avoid destroying uclasses with of-platdata-inst clk: sandbox: Move priv/plat data to a header file clk: fixed-rate: Export driver parts for OF_PLATDATA_INST clk: sandbox: Create a special fixed-rate driver sandbox: Create a new sandbox_noinst build test: Run sandbox_spl tests on sandbox_noinst azure/gitlab: Add tests for sandbox_noinst dm: core: Add an option to support SPL in read-only memory dm: core: Create a struct for device runtime info dm: core: Move flags to device-runtime info dm: core: Allow storing priv/plat data separately sandbox: Define a region for device priv/plat data dm: core: Use separate priv/plat data region x86: Define a region for device priv/plat data x86: apl: Fix the header order in pmc x86: apl: Tell of-platdata about a required header file x86: itss: Tidy up bind() for of-platdata-inst x86: Support a fake PCI device with of-platdata-inst x86: Don't include reset driver in SPL x86: coral: Drop ACPI properties from of-platdata x86: apl: Use read-only SPL and new of-platdata .azure-pipelines.yml | 3 + .gitlab-ci.yml | 10 +- arch/sandbox/cpu/os.c | 1 - arch/sandbox/cpu/u-boot-spl.lds| 8 + arch/sandbox/dts/sandbox.dtsi | 2 +- arch/sandbox/include/asm/clk.h | 24 +++ arch/x86/cpu/apollolake/Kconfig| 2 + arch/x86/cpu/apollolake/pmc.c | 2 +- arch/x86/cpu/apollolake/punit.c| 1 + arch/x86/cpu/intel_common/itss.c | 5 +- arch/x86/cpu/u-boot-spl.lds| 8 + arch/x86/dts/reset.dtsi| 2 +- arch/x86/lib/tpl.c | 1 + board/sandbox/MAINTAINERS | 7 + common/spl/Kconfig | 24 +++ configs/chromebook_coral_defconfig | 1 + configs/sandbox_noinst_defconfig | 229 + configs/sandbox_spl_defconfig | 1 + drivers/clk/clk_fixed_rate.c | 14 +- drivers/clk/clk_sandbox.c | 40 - drivers/clk/clk_sandbox_test.c | 6 - drivers/core/device.c | 83 --- drivers/core/root.c| 83 +-- drivers/core/uclass.c | 5 +- dts/Kconfig| 38 + include/asm-generic/global_data.h | 22 +++ include/asm-generic/sections.h | 3 + include/dm/device-internal.h | 26 include/dm/device.h| 38 - include/dm/root.h | 3 + include/dm/uclass-internal.h | 23 +++
Re: [PATCH 08/49] dm: Rename U_BOOT_DEVICE() to U_BOOT_DRVINFO()
On 12/29/20 12:34 AM, Simon Glass wrote: The current macro is a misnomer since it does not declare a device directly. Instead, it declares driver_info record which U-Boot uses at runtime to create a device. The distinction seems somewhat minor most of the time, but is becomes quite confusing when we actually want to declare a device, with of-platdata. We are left trying to distinguish between a device which isn't actually device, and a device that is (perhaps an 'instance'?) It seems better to rename this macro to describe what it actually is. The macros is not widely used, since boards should use devicetree to declare devices. Rename it to U_BOOT_DRVINFO(), which indicates clearly that this is declaring a new driver_info record, not a device. Signed-off-by: Simon Glass I couldn't agree more. Reviewed-by: Walter Lozano Thanks, --- .../mach-at91/arm926ejs/at91sam9260_devices.c | 2 +- .../arm926ejs/at91sam9m10g45_devices.c| 2 +- arch/arm/mach-imx/mx6/soc.c | 2 +- arch/arm/mach-imx/mx7/soc.c | 2 +- arch/arm/mach-lpc32xx/devices.c | 4 +- arch/arm/mach-omap2/am33xx/board.c| 10 +-- arch/arm/mach-omap2/omap3/board.c | 2 +- arch/arm/mach-tegra/board.c | 2 +- arch/arm/mach-tegra/board2.c | 2 +- board/armltd/integrator/integrator.c | 2 +- board/armltd/total_compute/total_compute.c| 2 +- board/armltd/vexpress64/vexpress64.c | 2 +- board/bluewater/gurnard/gurnard.c | 2 +- board/bluewater/snapper9260/snapper9260.c | 2 +- board/cadence/xtfpga/xtfpga.c | 4 +- board/cavium/thunderx/thunderx.c | 4 +- board/compulab/cm_fx6/cm_fx6.c| 2 +- board/davinci/da8xxevm/omapl138_lcdk.c| 4 +- board/freescale/ls1012afrdm/eth.c | 4 +- board/freescale/ls1012aqds/eth.c | 4 +- board/freescale/ls1012ardb/eth.c | 4 +- board/freescale/lx2160a/lx2160a.c | 4 +- board/gateworks/gw_ventana/gw_ventana.c | 2 +- board/hisilicon/hikey/hikey.c | 4 +- board/hisilicon/hikey960/hikey960.c | 2 +- board/hisilicon/poplar/poplar.c | 2 +- board/isee/igep00x0/igep00x0.c| 2 +- board/lg/sniper/sniper.c | 2 +- board/nokia/rx51/rx51.c | 2 +- board/sandbox/sandbox.c | 2 +- board/siemens/corvus/board.c | 2 +- board/st/stv0991/stv0991.c| 2 +- board/sysam/amcore/amcore.c | 2 +- board/ti/am335x/board.c | 6 +- board/timll/devkit8000/devkit8000.c | 2 +- board/toradex/apalis_imx6/apalis_imx6.c | 2 +- .../toradex/colibri-imx6ull/colibri-imx6ull.c | 2 +- board/toradex/colibri_imx6/colibri_imx6.c | 2 +- board/toradex/colibri_pxa270/colibri_pxa270.c | 4 +- doc/driver-model/design.rst | 18 ++--- doc/driver-model/of-plat.rst | 12 ++-- doc/driver-model/remoteproc-framework.rst | 2 +- doc/driver-model/spi-howto.rst| 4 +- drivers/crypto/fsl/fsl_rsa.c | 2 +- drivers/crypto/rsa_mod_exp/mod_exp_sw.c | 2 +- drivers/demo/demo-pdata.c | 10 +-- drivers/gpio/imx_rgpio2p.c| 4 +- drivers/gpio/mxc_gpio.c | 2 +- drivers/remoteproc/sandbox_testproc.c | 2 +- drivers/rtc/emul_rtc.c| 2 +- drivers/serial/sandbox.c | 2 +- drivers/sysreset/sysreset_sandbox.c | 4 +- drivers/timer/sandbox_timer.c | 2 +- drivers/video/sunxi/sunxi_de2.c | 2 +- drivers/video/sunxi/sunxi_dw_hdmi.c | 2 +- drivers/video/sunxi/sunxi_lcd.c | 2 +- dts/Kconfig | 8 +-- include/dm/device.h | 4 +- include/dm/lists.h| 2 +- include/dm/platdata.h | 16 ++--- include/dm/platform_data/spi_pl022.h | 2 +- test/dm/core.c| 6 +- tools/dtoc/dtb_platdata.py| 8 +-- tools/dtoc/test_dtoc.py | 66 +-- 64 files changed, 148 insertions(+), 148 deletions(-) diff --git a/arch/arm/mach-at91/arm926ejs/at91sam9260_devices.c b/arch/arm/mach-at91/arm926ejs/at91sam9260_devices.c index 9d787197f35..c10571fa28a 100644 --- a/arch/arm/mach-at91/arm926ejs/at91sam9260_devices.c +++ b/arch/arm/mach-at91/arm926ejs/at91sam9260_devices.c @@ -219,7 +219,7 @@ static const struct at91_port_plat at91sam9260_plat[] = { { ATMEL_BASE_PIOC, "PC" }, }; -U_BOOT_DEVICES(at91sam9260_gpios) = { +U_BOOT_DRVINFOS(at91sam
Re: [PATCH 09/49] dm: Rename DM_GET_DEVICE() to DM_DRVINFO_GET()
On 12/29/20 12:34 AM, Simon Glass wrote: This does not get a device (struct udevice *) but a struct driver_info * so the name is confusing. Rename it accordingly. Since we plan to have several various of these macros, put GET at the end instead of the middle, so it is easier to spot the related macros. Signed-off-by: Simon Glass Reviewed-by: Walter Lozano Thanks, --- doc/driver-model/of-plat.rst | 2 +- include/dm/platdata.h| 10 +++--- tools/dtoc/dtb_platdata.py | 4 ++-- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/doc/driver-model/of-plat.rst b/doc/driver-model/of-plat.rst index 39e6295aa09..21b45f6a7e7 100644 --- a/doc/driver-model/of-plat.rst +++ b/doc/driver-model/of-plat.rst @@ -339,7 +339,7 @@ prevents them being used inadvertently. All usage must be bracketed with The dt-plat.c file contains the device declarations and is is built in spl/dt-plat.c. It additionally contains the definition of dm_populate_phandle_data() which is responsible of filling the phandle -information by adding references to U_BOOT_DRVINFO by using DM_GET_DEVICE +information by adding references to U_BOOT_DRVINFO by using DM_DRVINFO_GET The pylibfdt Python module is used to access the devicetree. diff --git a/include/dm/platdata.h b/include/dm/platdata.h index e2b16ce6e4e..df1f6abcc7e 100644 --- a/include/dm/platdata.h +++ b/include/dm/platdata.h @@ -74,27 +74,23 @@ struct driver_rt { /** * Get a pointer to a given device info given its name * - * With the declaration U_BOOT_DRVINFO(name), DM_GET_DEVICE(name) will return a + * With the declaration U_BOOT_DRVINFO(name), DM_DRVINFO_GET(name) will return a * pointer to the struct driver_info created by that declaration. * * if OF_PLATDATA is enabled, from this it is possible to use the @dev member of * struct driver_info to find the device pointer itself. * - * TODO(s...@chromium.org): U_BOOT_DRVINFO() tells U-Boot to create a device, so - * the naming seems sensible, but DM_GET_DEVICE() is a bit of misnomer, since it - * finds the driver_info record, not the device. - * * @__name: Driver name (C identifier, not a string. E.g. gpio7_at_ff7e) * @return struct driver_info * to the driver that created the device */ -#define DM_GET_DEVICE(__name) \ +#define DM_DRVINFO_GET(__name) \ ll_entry_get(struct driver_info, __name, driver_info) /** * dm_populate_phandle_data() - Populates phandle data in platda * * This populates phandle data with an U_BOOT_DRVINFO entry get by - * DM_GET_DEVICE. The implementation of this function will be done + * DM_DRVINFO_GET. The implementation of this function will be done * by dtoc when parsing dtb. */ void dm_populate_phandle_data(void); diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index ebe5132e143..1a6a511b015 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -811,8 +811,8 @@ class DtbPlatdata(): nodes_to_output.remove(node) # Define dm_populate_phandle_data() which will add the linking between -# nodes using DM_GET_DEVICE -# dtv_dmc_at_xxx.clocks[0].node = DM_GET_DEVICE(clock_controller_at_xxx) +# nodes using DM_DRVINFO_GET +# dtv_dmc_at_xxx.clocks[0].node = DM_DRVINFO_GET(clock_controller_at_xxx) self.buf('void dm_populate_phandle_data(void) {\n') self.buf('}\n')
Re: [PATCH 45/49] dm: of-platadata: Add option for device instantiation
On 12/29/20 12:35 AM, Simon Glass wrote: Add Kconfig options to support build-time device instantiation. When fully implemented, this will allow dtoc to create U-Boot devices (i.e. struct udevice records) at build time, thus reducing code space in SPL. For now this defaults to off, but will be enabled when the rest of the implementation is in place. Signed-off-by: Simon Glass --- dts/Kconfig | 23 +-- scripts/Makefile.spl | 4 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/dts/Kconfig b/dts/Kconfig index 71f50552e4f..e861ea48d01 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -355,15 +355,24 @@ config SPL_OF_PLATDATA compatible string, then adding platform data and U_BOOT_DRVINFO declarations for each node. See of-plat.txt for more information. +if SPL_OF_PLATDATA + config SPL_OF_PLATDATA_PARENT bool "Support parent information in devices" - depends on SPL_OF_PLATDATA default y help Generally it is useful to be able to access the parent of a device with of-platdata. To save space this can be disabled, but in that case dev_get_parent() will always return NULL; +config SPL_OF_PLATDATA_INST Not sure about the limits to config names, I understand that SPL_OF_PLATDATA_INST means SPL_OF_PLATDATA_INSTANCE? I'm not sure but maybe a better name could be SPL_OF_PLATDATA_BTIME_DEV. To be honest I find it difficult to come with nice name. + bool "Declare devices at build time" + help + Declare devices as udevice instances so that they do not need to be + bound when U-Boot starts. This can save time and code space. + +endif + config TPL_OF_PLATDATA bool "Generate platform data for use in TPL" depends on TPL_OF_CONTROL @@ -385,13 +394,23 @@ config TPL_OF_PLATDATA compatible string, then adding platform data and U_BOOT_DRVINFO declarations for each node. See of-plat.txt for more information. +if TPL_OF_PLATDATA + config TPL_OF_PLATDATA_PARENT bool "Support parent information in devices" - depends on TPL_OF_PLATDATA default y help Generally it is useful to be able to access the parent of a device with of-platdata. To save space this can be disabled, but in that case dev_get_parent() will always return NULL; +config TPL_OF_PLATDATA_INST Same here. Also I have a question, in which case TPL_OF_PLATDATA_INST and SPL_OF_PLATDATA_INST would have different values. Which is the value of decoupling the TPL and SPL behavior? Or is use for consistency with SPL_OF_PLATDATA_PARENT and TPL_OF_PLATDATA_PARENT (which also not sure if needed to have two different options). + bool "Declare devices at build time" + + help + Declare devices as udevice instances so that they do not need to be + bound when U-Boot starts. This can save time and code space. + +endif + endmenu diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 72c0ad82793..6ca33abf348 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -311,6 +311,10 @@ pythonpath = PYTHONPATH=scripts/dtc/pylibfdt DTOC_ARGS := $(pythonpath) $(srctree)/tools/dtoc/dtoc \ -d $(obj)/$(SPL_BIN).dtb -p $(SPL_NAME) +ifneq ($(CONFIG_$(SPL_TPL_)OF_PLATDATA_INST),) +DTOC_ARGS += -i +endif + quiet_cmd_dtoc = DTOC$@ cmd_dtoc = $(DTOC_ARGS) -c $(obj)/dts -C include/generated all
Re: [PATCH 5/5] dtoc: Tidy up Python style in dtb_platdata
Hi Simon, Thanks for this series. I've tried to test it but I had issues to apply it. I have tried in u-boot, both master and next, and u-boot-dm. Could you please point me to the right tree/version? On 9/11/20 00:36, Simon Glass wrote: Update this, mostly to add comments for argument and return types. It is probably still too early to use type hinting since it was introduced in 3.5. Signed-off-by: Simon Glass --- tools/dtoc/dtb_platdata.py | 71 ++ 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 0ef245397a9..6b10eabafb9 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -9,6 +9,8 @@ This supports converting device tree data to C structures definitions and static data. + +See doc/driver-model/of-plat.rst for more informaiton *information """ import collections @@ -19,9 +21,9 @@ import sys from dtoc import fdt from dtoc import fdt_util -from patman import tools -# When we see these properties we ignore them - i.e. do not create a structure member +# When we see these properties we ignore them - i.e. do not create a structure +# member PROP_IGNORE_LIST = [ '#address-cells', '#gpio-cells', @@ -69,9 +71,9 @@ def conv_name_to_c(name): (400ms for 1m calls versus 1000ms for the 're' version). Args: -name: Name to convert +name (str): Name to convert Return: -String containing the C version of this name +str: String containing the C version of this name """ new = name.replace('@', '_at_') new = new.replace('-', '_') @@ -83,11 +85,11 @@ def tab_to(num_tabs, line): """Append tabs to a line of text to reach a tab stop. Args: -num_tabs: Tab stop to obtain (0 = column 0, 1 = column 8, etc.) -line: Line of text to append to +num_tabs (int): Tab stop to obtain (0 = column 0, 1 = column 8, etc.) +line (str): Line of text to append to Returns: -line with the correct number of tabs appeneded. If the line already +str: line with the correct number of tabs appeneded. If the line already extends past that tab stop then a single space is appended. """ if len(line) >= num_tabs * 8: @@ -103,28 +105,31 @@ def get_value(ftype, value): For booleans this return 'true' Args: -type: Data type (fdt_util) -value: Data value, as a string of bytes +ftype (fdt.Type): Data type (fdt_util) +value (bytes): Data value, as a string of bytes + +Returns: +str: String representation of the value """ if ftype == fdt.Type.INT: return '%#x' % fdt_util.fdt32_to_cpu(value) elif ftype == fdt.Type.BYTE: ch = value[0] -return '%#x' % (ord(ch) if type(ch) == str else ch) +return '%#x' % (ord(ch) if isinstance(ch, str) else ch) elif ftype == fdt.Type.STRING: # Handle evil ACPI backslashes by adding another backslash before them. # So "\\_SB.GPO0" in the device tree effectively stays like that in C return '"%s"' % value.replace('\\', '') elif ftype == fdt.Type.BOOL: return 'true' -elif ftype == fdt.Type.INT64: +else: # ftype == fdt.Type.INT64: return '%#x' % value def get_compat_name(node): """Get the node's list of compatible string as a C identifiers Args: -node: Node object to check +node (fdt.Node): Node object to check Return: List of C identifiers for all the compatible strings """ @@ -213,7 +218,7 @@ class DtbPlatdata(object): file. Args: -fname: Filename to send output to, or '-' for stdout +fname (str): Filename to send output to, or '-' for stdout """ if fname == '-': self._outfile = sys.stdout @@ -224,7 +229,7 @@ class DtbPlatdata(object): """Output a string to the output file Args: -line: String to output +line (str): String to output """ self._outfile.write(line) @@ -232,7 +237,7 @@ class DtbPlatdata(object): """Buffer up a string to send later Args: -line: String to add to our 'buffer' list +line (str): String to add to our 'buffer' list """ self._lines.append(line) @@ -240,7 +245,7 @@ class DtbPlatdata(object): """Get the contents of the output buffer, and clear it Returns: -The output buffer, which is then cleared for future use +list(str): The output buffer, which is then cleared for future use """ lines = self._lines self._lines = [] @@ -263,9 +268,14 @@ class DtbPlatdata(object): or not. As an interim measure, use a list of
Re: [PATCH] imx: mx6cuboxi: Disable CONFIG_IMX_THERMAL
On 29/10/20 12:19, Tom Rini wrote: On Thu, Oct 29, 2020 at 07:14:55AM +0200, Baruch Siach wrote: Hi Simon, Adding Walter to Cc. On Thu, Oct 29 2020, Simon Glass wrote: This feature is incompatble with of-platdata since it uses the U_BOOT_DEVICE() macro. With of-platdata the only devices permitted are those created by dtoc. Drop this option for now, until the driver can be corrected. As I understand, of-platdata is only enabled in SPL. Is there a way to limit imx_thermal drop to SPL? The thermal driver in not very useful in SPL anyway, right? As you pointed of-platdata is intended for TPL/SPL to avoid the overhead of DTB and libraries. If this option is enabled and the driver does not support it, the device won't be created in TPL/SPL but is should on U-Boot. In order to limit the drop to SPL can CONFIG_SPL_BUILD be used? Regarding the usefulness of im_thermal in SPL I can't comment. Well, sigh, step one is to finish the migration to Kconfig for IMX_THERMAL as it has a symbol, but it's still set in a ton of board config files. We only build thermal in SPL when SPL_THERMAL is set, so I assume this is about trying to use of-platdata in U-Boot proper too, to keep / further reduce the size of U-Boot itself to stay within size limits. Regards, Walter
Re: [RFC 3/4] dtoc: add support for generate stuct udevice_id
Hi Simon, On 6/9/20 22:44, Simon Glass wrote: Hi Walter, On Fri, 7 Aug 2020 at 11:23, Walter Lozano wrote: Hi Simon On 7/8/20 13:23, Simon Glass wrote: Hi Walter, On Wed, 29 Jul 2020 at 10:00, Walter Lozano wrote: Hi Simon, On 28/7/20 23:42, Simon Glass wrote: Hi Walter, On Sun, 26 Jul 2020 at 20:16, Walter Lozano wrote: Hi Simon, On 26/7/20 11:53, Simon Glass wrote: Hi Walter, On Tue, 7 Jul 2020 at 08:08, Walter Lozano wrote: Hi Simon On 6/7/20 16:21, Simon Glass wrote: Hi Walter, On Fri, 19 Jun 2020 at 15:12, Walter Lozano wrote: Based on several reports there is an increasing concern in the impact of adding additional features to drivers based on compatible strings. A good example of this situation is found in [1]. In order to reduce this impact and as an initial step for further reduction, propose a new way to declare compatible strings, which allows to only include the useful ones. What are the useful ones? The useful ones would be those that are used by the selected DTB by the current configuration. The idea of this patch is to declare all the possible compatible strings in a way that dtoc can generate code for only those which are going to be used, and in this way avoid lots of #ifdef like the ones shows in http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ The idea is to define compatible strings in a way to be easily parsed by dtoc, which will be responsible to build struct udevice_id [] based on the compatible strings present in the dtb. Additional features can be easily added, such as define constants depending on the presence of compatible strings, which allows to enable code blocks only in such cases without the need of adding additional configuration options. [1] http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 32 1 file changed, 32 insertions(+) I think dtoc should be able to parse the compatible strings as they are today - e.g. see the tiny-dm stuff. Yes, I agree. My idea is that dtoc parses compatible strings as they are today but also in this new way. The reason for this is to allow dtoc to generate the code to include the useful compatible strings. Of course, this only makes sense if the idea of generating the compatible string associated code is accepted. What do you think? I think this is useful and better than using #ifdef in the source code for this sort of thing. We need a way to specify the driver_data value as well, right? Yes, I agree, it is better than #ifdef and c/ould give us some extra functionality. What doe you mean by driver_data value? Are you referring to the data field? like static struct esdhc_soc_data usdhc_imx7d_data = { .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200 | ESDHC_FLAG_HS400, }; Actually I was talking about the .data member in struct udevice_id. So my example is correct, as usdhc_imx7d_data is the value for .data in one case as shown bellow. If that is the case, I was thinking in defining a constant when specific compatible strings are enabled by dtoc, based in the above case #ifdef FSL_ESDHC_IMX_V2 static struct esdhc_soc_data usdhc_imx7d_data = { .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200 | ESDHC_FLAG_HS400, }; #endif COMPATIBLE(FSL_ESDHC, "fsl,imx7d-usdhc", _imx7d_data, FSL_ESDHC_IMX_V2) So when dtoc parses COMPATIBLE and determines that compatible "fsl,imx7d-usdhc" should be added it also defines FSL_ESDHC_IMX_V2. I think we can put that data in the dt-platdata.c file perhaps. I thought the same at the beginning, but then I changed my mind, because 1- in order to work dt-platdata.c will need to include several different .h, in this example, only for fsl_esdhc_imx to work, we will need to include fsl_esdhc_imx.h where all the flags are defined. Yes I hit that problem with the tiny-dm experiment and ended up adding a macro to specify the header. I haven't seen that. I will check it. Thanks. Do you need FSL_ESDHC_IMX_V2? Is it just to avoid a warning about usdhc_imx7d_data not being used? If so, we could use _maybe_unused Well, it is not that I really need it, but I try to give the possibility to add some #ifdef or similar based on compatible strings, the usdhc_imx7d_data was just an example. A more interesting example could be some code that makes sense only on specific "compatible string" cases and using #ifdef or if would save some bytes in other cases. 2- it case we use #define to avoid having to include several different .h probably the errors will be more difficult to catch/debug Yes we would have to include the real header, not just copy bits out of it
Re: [RFC PATCH v2 0/3] RFC: tiny-dm: Proposal for using driver model in SPL
Hi Simon, On 16/8/20 00:06, Simon Glass wrote: Hi Walter, On Sun, 26 Jul 2020 at 20:45, Walter Lozano wrote: Hi Simon, On 10/7/20 01:12, Walter Lozano wrote: Hi Simon, On 2/7/20 18:10, Simon Glass wrote: This series provides a proposed enhancement to driver model to reduce overhead in SPL. These patches should not be reviewed other than to comment on the approach. The code is all lumped together in a few patches and so cannot be applied as is. For now, the source tree is available at: https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/dtoc-working Comments welcome! Benefits (good news) As an example of the impact of tiny-dm, chromebook_jerry is converted to use it. This shows approximately a 30% reduction in code and data size and a 85% reduction in malloc() space over of-platdata: text databssdechexfilename 25248 1836 12 27096 69d8 spl/u-boot-spl (original with DT) 19727 3436 12 23175 5a87 spl/u-boot-spl (OF_PLATDATA) 78%187%100% 86% as %age of original 13784 1408 12 15204 3b64 spl/u-boot-spl (SPL_TINY) 70% 41%100% 66% as %age of platdata 55% 77%100% 56% as %age of original SPL malloc() usage drops from 944 bytes (OF_PLATDATA) to 116 (SPL_TINY). Overall the 'overhead' of tiny-dm is much less than the full driver model. Code size is currently about 600 bytes for these functions on Thumb2: 0054 T tiny_dev_probe 0034 T tiny_dev_get_by_drvdata 0024 T tiny_dev_find 001a T tiny_dev_get 003c T tinydev_alloc_data 002a t tinydev_lookup_data 0022 T tinydev_ensure_data 0014 T tinydev_get_data 0004 T tinydev_get_parent Effort (bad news) - Unfortunately it is quite a bit of work to convert drivers over to tiny-dm. First, the of-platdata conversion must be done. But on top of that, tiny-dm needs entirely separate code for dealing with devices. This means that instead of 'struct udevice' and 'struct uclass' there is just 'struct tinydev'. Each driver and uclass must be modified to support both, pulling common code into internal static functions. Another option -- Note: It is assumed that any board that is space-contrained should use of-platdata in SPL (see doc/driver-model/of-plat.rst). This is shown to reduce device-tree overhead by approximately 4KB. Designing tiny-dm has suggested a number of things that could be changed in the current driver model to make it more space-efficient for TPL and SPL. The ones with least impact on driver code are (CS=reduces code size, DS=reduces data size): CS - drop driver_bind() and create devices (struct udevice) at build-time CS - allocate all device- and uclass-private data at build-time CS - remove all but the basic operations for each uclass (e.g. SPI flash only supports reading) DS - use 8-bit indexes instead of 32/64-bit pointers for device pointers possible since these are created at build-time) DS - use singly-linked lists DS - use 16-bit offsets to private data, instead of 32/64-bit pointers (possible since it is all in SRAM relative to malloc() base, presumably word-aligned and < 256KB) DS - move private pointers into a separate data structure so that NULLs are not stored CS / DS - Combine req_seq and seq and calculate the new value at build-time More difficult are: DS - drop some of the lesser-used driver and uclass methods DS - drop all uclass methods except init() DS - drop all driver methods except probe() CS / DS - drop uclasses and require drivers to manually call uclass functions Even with all of this we would not reach tiny-dm and it would muddy up the driver-model datas structures. But we might come close to tiny-dm on size and there are some advantages: - much of the benefit would apply to all boards that use of-platdata (i.e. with very little effort on behalf of board maintainers) - the impact on the code base is much less (we keep a single, unified driver mode in SPL and U-Boot proper) Overall I think it is worth looking at this option. While it doesn't have the 'nuclear' impact of tiny-dm, neither does it mess with the U-Boot driver code as much and it is easier to learn. Thanks for your hard work on this topic. I think that there is great value in this research and in this conclusion. It is clear that there two different approaches, but I feel that the improvement to the current DM implementation would have a higher impact in the community. Since the first version of this proposal I have been thinking in a solution that takes some of the advantages of tiny-dm idea but that does not require so much effort. This seems to be aligned with what
Re: [RFC 3/4] dtoc: add support for generate stuct udevice_id
Hi Simon On 7/8/20 13:23, Simon Glass wrote: Hi Walter, On Wed, 29 Jul 2020 at 10:00, Walter Lozano wrote: Hi Simon, On 28/7/20 23:42, Simon Glass wrote: Hi Walter, On Sun, 26 Jul 2020 at 20:16, Walter Lozano wrote: Hi Simon, On 26/7/20 11:53, Simon Glass wrote: Hi Walter, On Tue, 7 Jul 2020 at 08:08, Walter Lozano wrote: Hi Simon On 6/7/20 16:21, Simon Glass wrote: Hi Walter, On Fri, 19 Jun 2020 at 15:12, Walter Lozano wrote: Based on several reports there is an increasing concern in the impact of adding additional features to drivers based on compatible strings. A good example of this situation is found in [1]. In order to reduce this impact and as an initial step for further reduction, propose a new way to declare compatible strings, which allows to only include the useful ones. What are the useful ones? The useful ones would be those that are used by the selected DTB by the current configuration. The idea of this patch is to declare all the possible compatible strings in a way that dtoc can generate code for only those which are going to be used, and in this way avoid lots of #ifdef like the ones shows in http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ The idea is to define compatible strings in a way to be easily parsed by dtoc, which will be responsible to build struct udevice_id [] based on the compatible strings present in the dtb. Additional features can be easily added, such as define constants depending on the presence of compatible strings, which allows to enable code blocks only in such cases without the need of adding additional configuration options. [1] http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 32 1 file changed, 32 insertions(+) I think dtoc should be able to parse the compatible strings as they are today - e.g. see the tiny-dm stuff. Yes, I agree. My idea is that dtoc parses compatible strings as they are today but also in this new way. The reason for this is to allow dtoc to generate the code to include the useful compatible strings. Of course, this only makes sense if the idea of generating the compatible string associated code is accepted. What do you think? I think this is useful and better than using #ifdef in the source code for this sort of thing. We need a way to specify the driver_data value as well, right? Yes, I agree, it is better than #ifdef and c/ould give us some extra functionality. What doe you mean by driver_data value? Are you referring to the data field? like static struct esdhc_soc_data usdhc_imx7d_data = { .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200 | ESDHC_FLAG_HS400, }; Actually I was talking about the .data member in struct udevice_id. So my example is correct, as usdhc_imx7d_data is the value for .data in one case as shown bellow. If that is the case, I was thinking in defining a constant when specific compatible strings are enabled by dtoc, based in the above case #ifdef FSL_ESDHC_IMX_V2 static struct esdhc_soc_data usdhc_imx7d_data = { .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200 | ESDHC_FLAG_HS400, }; #endif COMPATIBLE(FSL_ESDHC, "fsl,imx7d-usdhc", _imx7d_data, FSL_ESDHC_IMX_V2) So when dtoc parses COMPATIBLE and determines that compatible "fsl,imx7d-usdhc" should be added it also defines FSL_ESDHC_IMX_V2. I think we can put that data in the dt-platdata.c file perhaps. I thought the same at the beginning, but then I changed my mind, because 1- in order to work dt-platdata.c will need to include several different .h, in this example, only for fsl_esdhc_imx to work, we will need to include fsl_esdhc_imx.h where all the flags are defined. Yes I hit that problem with the tiny-dm experiment and ended up adding a macro to specify the header. I haven't seen that. I will check it. Thanks. Do you need FSL_ESDHC_IMX_V2? Is it just to avoid a warning about usdhc_imx7d_data not being used? If so, we could use _maybe_unused Well, it is not that I really need it, but I try to give the possibility to add some #ifdef or similar based on compatible strings, the usdhc_imx7d_data was just an example. A more interesting example could be some code that makes sense only on specific "compatible string" cases and using #ifdef or if would save some bytes in other cases. 2- it case we use #define to avoid having to include several different .h probably the errors will be more difficult to catch/debug Yes we would have to include the real header, not just copy bits out of it. Yes, for that reason I feel it could lead to more issues than benefits. However, it is only a personal op
[PATCH 0/2] drivers: use of_match_ptr to avoid references when OF_PLATDATA is used
In order to save extra bytes when OF_PLATDATA is enabled avoid referencing compatible strings and additional functions by extending the use of of_match_ptr taking into account this configuration. Walter Lozano (2): core: improve of_match_ptr with OF_PLATDATA drivers: use of_match_ptr to avoid references when OF_PLATDATA is used drivers/clk/clk_fixed_factor.c| 2 +- drivers/clk/clk_fixed_rate.c | 2 +- drivers/clk/rockchip/clk_px30.c | 4 ++-- drivers/clk/rockchip/clk_rk3188.c | 2 +- drivers/clk/rockchip/clk_rk3288.c | 2 +- drivers/clk/rockchip/clk_rk3308.c | 2 +- drivers/clk/rockchip/clk_rk3368.c | 2 +- drivers/clk/rockchip/clk_rk3399.c | 4 ++-- drivers/core/simple-bus.c | 2 +- drivers/core/syscon-uclass.c | 2 +- drivers/gpio/mxs_gpio.c | 6 ++ drivers/mmc/ftsdc010_mci.c| 2 +- drivers/mmc/mxsmmc.c | 6 ++ drivers/mmc/rockchip_dw_mmc.c | 4 ++-- drivers/mmc/rockchip_sdhci.c | 2 +- drivers/pinctrl/intel/pinctrl_apl.c | 2 +- drivers/pinctrl/rockchip/pinctrl-px30.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3328.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3399.c | 2 +- drivers/ram/rockchip/dmc-rk3368.c | 2 +- drivers/ram/rockchip/sdram_rk3188.c | 2 +- drivers/ram/rockchip/sdram_rk322x.c | 2 +- drivers/ram/rockchip/sdram_rk3288.c | 2 +- drivers/ram/rockchip/sdram_rk3328.c | 2 +- drivers/ram/rockchip/sdram_rk3399.c | 2 +- drivers/spi/ich.c | 2 +- drivers/spi/mxs_spi.c | 6 ++ drivers/spi/rk_spi.c | 2 +- drivers/timer/rockchip_timer.c| 2 +- include/dm/device.h | 2 +- 30 files changed, 36 insertions(+), 42 deletions(-) -- 2.20.1
[PATCH 1/2] core: improve of_match_ptr with OF_PLATDATA
Currently of_match_ptr is used to avoid referencing compatible strings when OF_CONTROL is not enabled. This behaviour could be improved by taking into account also OF_PLATDATA, as when this configuration is enabled the compatible strings are not used at all. Signed-off-by: Walter Lozano --- include/dm/device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/dm/device.h b/include/dm/device.h index 953706cf52..ac3b6c1b8a 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -197,7 +197,7 @@ struct udevice_id { ulong data; }; -#if CONFIG_IS_ENABLED(OF_CONTROL) +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) #define of_match_ptr(_ptr) (_ptr) #else #define of_match_ptr(_ptr) NULL -- 2.20.1
[PATCH 2/2] drivers: use of_match_ptr to avoid references when OF_PLATDATA is used
As when OF_PLATDATA is used compatible strings are not used at all remove their reference to save extra bytes. Also in the cases where this was done with #define change to of_match_ptr to improve readability. Signed-off-by: Walter Lozano --- drivers/clk/clk_fixed_factor.c| 2 +- drivers/clk/clk_fixed_rate.c | 2 +- drivers/clk/rockchip/clk_px30.c | 4 ++-- drivers/clk/rockchip/clk_rk3188.c | 2 +- drivers/clk/rockchip/clk_rk3288.c | 2 +- drivers/clk/rockchip/clk_rk3308.c | 2 +- drivers/clk/rockchip/clk_rk3368.c | 2 +- drivers/clk/rockchip/clk_rk3399.c | 4 ++-- drivers/core/simple-bus.c | 2 +- drivers/core/syscon-uclass.c | 2 +- drivers/gpio/mxs_gpio.c | 6 ++ drivers/mmc/ftsdc010_mci.c| 2 +- drivers/mmc/mxsmmc.c | 6 ++ drivers/mmc/rockchip_dw_mmc.c | 4 ++-- drivers/mmc/rockchip_sdhci.c | 2 +- drivers/pinctrl/intel/pinctrl_apl.c | 2 +- drivers/pinctrl/rockchip/pinctrl-px30.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3328.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3399.c | 2 +- drivers/ram/rockchip/dmc-rk3368.c | 2 +- drivers/ram/rockchip/sdram_rk3188.c | 2 +- drivers/ram/rockchip/sdram_rk322x.c | 2 +- drivers/ram/rockchip/sdram_rk3288.c | 2 +- drivers/ram/rockchip/sdram_rk3328.c | 2 +- drivers/ram/rockchip/sdram_rk3399.c | 2 +- drivers/spi/ich.c | 2 +- drivers/spi/mxs_spi.c | 6 ++ drivers/spi/rk_spi.c | 2 +- drivers/timer/rockchip_timer.c| 2 +- 29 files changed, 35 insertions(+), 41 deletions(-) diff --git a/drivers/clk/clk_fixed_factor.c b/drivers/clk/clk_fixed_factor.c index cf9c4ae367..f4171464f6 100644 --- a/drivers/clk/clk_fixed_factor.c +++ b/drivers/clk/clk_fixed_factor.c @@ -65,7 +65,7 @@ static const struct udevice_id clk_fixed_factor_match[] = { U_BOOT_DRIVER(clk_fixed_factor) = { .name = "fixed_factor_clock", .id = UCLASS_CLK, - .of_match = clk_fixed_factor_match, + .of_match = of_match_ptr(clk_fixed_factor_match), .ofdata_to_platdata = clk_fixed_factor_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct clk_fixed_factor), .ops = _fixed_factor_ops, diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c index 2c20eddb0b..cfffb18553 100644 --- a/drivers/clk/clk_fixed_rate.c +++ b/drivers/clk/clk_fixed_rate.c @@ -49,7 +49,7 @@ static const struct udevice_id clk_fixed_rate_match[] = { U_BOOT_DRIVER(clk_fixed_rate) = { .name = "fixed_rate_clock", .id = UCLASS_CLK, - .of_match = clk_fixed_rate_match, + .of_match = of_match_ptr(clk_fixed_rate_match), .ofdata_to_platdata = clk_fixed_rate_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct clk_fixed_rate), .ops = _fixed_rate_ops, diff --git a/drivers/clk/rockchip/clk_px30.c b/drivers/clk/rockchip/clk_px30.c index 71916dbf3b..014ff51bad 100644 --- a/drivers/clk/rockchip/clk_px30.c +++ b/drivers/clk/rockchip/clk_px30.c @@ -1479,7 +1479,7 @@ static const struct udevice_id px30_clk_ids[] = { U_BOOT_DRIVER(rockchip_px30_cru) = { .name = "rockchip_px30_cru", .id = UCLASS_CLK, - .of_match = px30_clk_ids, + .of_match = of_match_ptr(px30_clk_ids), .priv_auto_alloc_size = sizeof(struct px30_clk_priv), .ofdata_to_platdata = px30_clk_ofdata_to_platdata, .ops= _clk_ops, @@ -1626,7 +1626,7 @@ static const struct udevice_id px30_pmuclk_ids[] = { U_BOOT_DRIVER(rockchip_px30_pmucru) = { .name = "rockchip_px30_pmucru", .id = UCLASS_CLK, - .of_match = px30_pmuclk_ids, + .of_match = of_match_ptr(px30_pmuclk_ids), .priv_auto_alloc_size = sizeof(struct px30_pmuclk_priv), .ofdata_to_platdata = px30_pmuclk_ofdata_to_platdata, .ops= _pmuclk_ops, diff --git a/drivers/clk/rockchip/clk_rk3188.c b/drivers/clk/rockchip/clk_rk3188.c index aacc8cf2d1..3c9049ad67 100644 --- a/drivers/clk/rockchip/clk_rk3188.c +++ b/drivers/clk/rockchip/clk_rk3188.c @@ -615,7 +615,7 @@ static const struct udevice_id rk3188_clk_ids[] = { U_BOOT_DRIVER(rockchip_rk3188_cru) = { .name = "rockchip_rk3188_cru", .id = UCLASS_CLK, - .of_match = rk3188_clk_ids, + .of_match = of_match_ptr(rk3188_clk_ids), .priv_auto_alloc_size = sizeof(struct rk3188_clk_priv), .platdata_auto_alloc_size = sizeof(struct rk3188_clk_plat), .ops= _clk_ops, diff --git a/drivers/clk/rockchip/clk_rk3288.c b/drivers/clk/rockchip/clk_rk3288.c index a1dd642eef..6914c31e66 10
Re: [RFC 3/4] dtoc: add support for generate stuct udevice_id
Hi Simon, On 28/7/20 23:42, Simon Glass wrote: Hi Walter, On Sun, 26 Jul 2020 at 20:16, Walter Lozano wrote: Hi Simon, On 26/7/20 11:53, Simon Glass wrote: Hi Walter, On Tue, 7 Jul 2020 at 08:08, Walter Lozano wrote: Hi Simon On 6/7/20 16:21, Simon Glass wrote: Hi Walter, On Fri, 19 Jun 2020 at 15:12, Walter Lozano wrote: Based on several reports there is an increasing concern in the impact of adding additional features to drivers based on compatible strings. A good example of this situation is found in [1]. In order to reduce this impact and as an initial step for further reduction, propose a new way to declare compatible strings, which allows to only include the useful ones. What are the useful ones? The useful ones would be those that are used by the selected DTB by the current configuration. The idea of this patch is to declare all the possible compatible strings in a way that dtoc can generate code for only those which are going to be used, and in this way avoid lots of #ifdef like the ones shows in http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ The idea is to define compatible strings in a way to be easily parsed by dtoc, which will be responsible to build struct udevice_id [] based on the compatible strings present in the dtb. Additional features can be easily added, such as define constants depending on the presence of compatible strings, which allows to enable code blocks only in such cases without the need of adding additional configuration options. [1] http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 32 1 file changed, 32 insertions(+) I think dtoc should be able to parse the compatible strings as they are today - e.g. see the tiny-dm stuff. Yes, I agree. My idea is that dtoc parses compatible strings as they are today but also in this new way. The reason for this is to allow dtoc to generate the code to include the useful compatible strings. Of course, this only makes sense if the idea of generating the compatible string associated code is accepted. What do you think? I think this is useful and better than using #ifdef in the source code for this sort of thing. We need a way to specify the driver_data value as well, right? Yes, I agree, it is better than #ifdef and c/ould give us some extra functionality. What doe you mean by driver_data value? Are you referring to the data field? like static struct esdhc_soc_data usdhc_imx7d_data = { .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200 | ESDHC_FLAG_HS400, }; Actually I was talking about the .data member in struct udevice_id. So my example is correct, as usdhc_imx7d_data is the value for .data in one case as shown bellow. If that is the case, I was thinking in defining a constant when specific compatible strings are enabled by dtoc, based in the above case #ifdef FSL_ESDHC_IMX_V2 static struct esdhc_soc_data usdhc_imx7d_data = { .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200 | ESDHC_FLAG_HS400, }; #endif COMPATIBLE(FSL_ESDHC, "fsl,imx7d-usdhc", _imx7d_data, FSL_ESDHC_IMX_V2) So when dtoc parses COMPATIBLE and determines that compatible "fsl,imx7d-usdhc" should be added it also defines FSL_ESDHC_IMX_V2. I think we can put that data in the dt-platdata.c file perhaps. I thought the same at the beginning, but then I changed my mind, because 1- in order to work dt-platdata.c will need to include several different .h, in this example, only for fsl_esdhc_imx to work, we will need to include fsl_esdhc_imx.h where all the flags are defined. 2- it case we use #define to avoid having to include several different .h probably the errors will be more difficult to catch/debug What do you think? This is alsoAs I comment you in the tread about tiny-dm I think that we can save some space following your suggestions, and for instance implement Re naming, perhaps DT_COMPAT() might be better than COMPATIBLE()? Or even a name that indicates that it is optional, like DT_OPT_COMPAT() ? I totally agree, naming is very important, and DT_COMPAT() is much better. What I don't fully understand is what are the cases for DT_OPT_COMPAT(), could you please clarify? It's just an alternative name, with OPT meaning optional. But I think we can leave out the OPT. Thanks for clarifying. Regards, Walter
[PATCH v3 4/6] mmc: fsl_esdhc_imx: add CD support when OF_PLATDATA is enabled
After enabling OF_PLATDATA support to both MMC and GPIO drivers add the support for card detection. Signed-off-by: Walter Lozano Reviewed-by: Simon Glass --- Changes in v3: - Change #ifdef to if when possible drivers/mmc/fsl_esdhc_imx.c | 25 - 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 2713682cf7..788677984b 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1498,7 +1498,30 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->bus_width = 4; else priv->bus_width = 1; - priv->non_removable = 1; + + if (dtplat->non_removable) + priv->non_removable = 1; + else + priv->non_removable = 0; + + if (CONFIG_IS_ENABLED(DM_GPIO) && !priv->non_removable) { + struct udevice *gpiodev; + struct driver_info *info; + + info = (struct driver_info *)dtplat->cd_gpios->node; + + ret = device_get_by_driver_info(info, ); + + if (ret) + return ret; + + ret = gpio_dev_request_index(gpiodev, gpiodev->name, "cd-gpios", +dtplat->cd_gpios->arg[0], GPIOD_IS_IN, +dtplat->cd_gpios->arg[1], >cd_gpio); + + if (ret) + return ret; + } #endif if (data) -- 2.20.1
Re: [PATCH v2 3/6] gpio: mxc_gpio: add OF_PLATDATA support
Hi Stefano, On 27/7/20 09:10, Stefano Babic wrote: Hi Walter, On 22.07.20 15:14, Walter Lozano wrote: Continuing with the OF_PLATADATA support for iMX6 to reduce SPL footprint, add it to mxc_gpio. Thanks to this, it will be possible to enable card detection on MMC driver. Signed-off-by: Walter Lozano --- This conflicts with commit 6103e570cdd0d0e854b071ee17b14589356a82bd Author: Ye Li Date: Tue Jun 9 20:29:51 2020 -0700 gpio: mxc_gpio: Improve to use ofdata_to_platdata already mainlined. Can you take a look ? Thanks ! Sure, thanks for pointing at the issue. I've already sent a new version. Regards, Walter (no changes since v1) drivers/gpio/mxc_gpio.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 316dcc757b..fc49b5b577 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -13,6 +13,8 @@ #include #include #include +#include +#include enum mxc_gpio_direction { MXC_GPIO_DIRECTION_IN, @@ -22,6 +24,10 @@ enum mxc_gpio_direction { #define GPIO_PER_BANK 32 struct mxc_gpio_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + /* Put this first since driver model will copy the data here */ + struct dtd_gpio_mxc dtplat; +#endif int bank_index; struct gpio_regs *regs; }; @@ -306,8 +312,16 @@ static int mxc_gpio_bind(struct udevice *dev) * is statically initialized in U_BOOT_DEVICES.Here * will return. */ - if (plat) + + if (plat) { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dtd_gpio_mxc *dtplat = >dtplat; + + plat->regs = map_sysmem(dtplat->reg[0], dtplat->reg[1]); + plat->bank_index = dev->req_seq; +#endif return 0; + } addr = devfdt_get_addr(dev); if (addr == FDT_ADDR_T_NONE) @@ -350,6 +364,8 @@ U_BOOT_DRIVER(gpio_mxc) = { .bind = mxc_gpio_bind, }; +U_BOOT_DRIVER_ALIAS(gpio_mxc, fsl_imx6q_gpio) + #if !CONFIG_IS_ENABLED(OF_CONTROL) static const struct mxc_gpio_plat mxc_plat[] = { { 0, (struct gpio_regs *)GPIO1_BASE_ADDR },
[PATCH v3 5/6] drivers: rename more drivers to match compatible string
Continuing with the approach in commit rename additional drivers to allow the OF_PLATDATA support. Signed-off-by: Walter Lozano Reviewed-by: Simon Glass --- (no changes since v1) drivers/pinctrl/nxp/pinctrl-imx6.c | 6 -- drivers/video/imx/mxc_ipuv3_fb.c | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c index aafa3057ad..84004e5921 100644 --- a/drivers/pinctrl/nxp/pinctrl-imx6.c +++ b/drivers/pinctrl/nxp/pinctrl-imx6.c @@ -41,8 +41,8 @@ static const struct udevice_id imx6_pinctrl_match[] = { { /* sentinel */ } }; -U_BOOT_DRIVER(imx6_pinctrl) = { - .name = "imx6-pinctrl", +U_BOOT_DRIVER(fsl_imx6q_iomuxc) = { + .name = "fsl_imx6q_iomuxc", .id = UCLASS_PINCTRL, .of_match = of_match_ptr(imx6_pinctrl_match), .probe = imx6_pinctrl_probe, @@ -51,3 +51,5 @@ U_BOOT_DRIVER(imx6_pinctrl) = { .ops = _pinctrl_ops, .flags = DM_FLAG_PRE_RELOC, }; + +U_BOOT_DRIVER_ALIAS(fsl_imx6q_iomuxc, fsl_imx6dl_iomuxc) diff --git a/drivers/video/imx/mxc_ipuv3_fb.c b/drivers/video/imx/mxc_ipuv3_fb.c index 587d62f2d8..492bc3e829 100644 --- a/drivers/video/imx/mxc_ipuv3_fb.c +++ b/drivers/video/imx/mxc_ipuv3_fb.c @@ -660,8 +660,8 @@ static const struct udevice_id ipuv3_video_ids[] = { { } }; -U_BOOT_DRIVER(ipuv3_video) = { - .name = "ipuv3_video", +U_BOOT_DRIVER(fsl_imx6q_ipu) = { + .name = "fsl_imx6q_ipu", .id = UCLASS_VIDEO, .of_match = ipuv3_video_ids, .bind = ipuv3_video_bind, -- 2.20.1
[PATCH v3 6/6] mx6cuboxi: enable OF_PLATDATA
As both MMC and GPIO driver now supports OF_PLATDATA, enable it in defconfig in order to reduce the SPL footprint. After applying this setting the SPL reduction is 5 KB, which partially compensates the increment due to DM. Signed-off-by: Walter Lozano Reviewed-by: Simon Glass --- (no changes since v2) Changes in v2: - Improve commit message with footprint reduction configs/mx6cuboxi_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig index d2378fa630..9086d6d0da 100644 --- a/configs/mx6cuboxi_defconfig +++ b/configs/mx6cuboxi_defconfig @@ -42,6 +42,7 @@ CONFIG_OF_CONTROL=y CONFIG_SPL_OF_CONTROL=y CONFIG_OF_LIST="imx6dl-hummingboard2-emmc-som-v15 imx6q-hummingboard2-emmc-som-v15" CONFIG_MULTI_DTB_FIT=y +CONFIG_SPL_OF_PLATDATA=y CONFIG_ENV_IS_IN_MMC=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y -- 2.20.1
[PATCH v3 3/6] gpio: mxc_gpio: add OF_PLATDATA support
Continuing with the OF_PLATADATA support for iMX6 to reduce SPL footprint, add it to mxc_gpio. Thanks to this, it will be possible to enable card detection on MMC driver. Signed-off-by: Walter Lozano --- Changes in v3: - Rework to resolve merge conflicts drivers/gpio/mxc_gpio.c | 26 +- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index a16f5719ed..e7585fce7b 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -13,6 +13,8 @@ #include #include #include +#include +#include enum mxc_gpio_direction { MXC_GPIO_DIRECTION_IN, @@ -22,6 +24,10 @@ enum mxc_gpio_direction { #define GPIO_PER_BANK 32 struct mxc_gpio_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + /* Put this first since driver model will copy the data here */ + struct dtd_gpio_mxc dtplat; +#endif int bank_index; struct gpio_regs *regs; }; @@ -280,6 +286,12 @@ static int mxc_gpio_probe(struct udevice *dev) int banknum; char name[18], *str; +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dtd_gpio_mxc *dtplat = >dtplat; + + plat->regs = map_sysmem(dtplat->reg[0], dtplat->reg[1]); +#endif + banknum = plat->bank_index; if (IS_ENABLED(CONFIG_ARCH_IMX8)) sprintf(name, "GPIO%d_", banknum); @@ -297,14 +309,16 @@ static int mxc_gpio_probe(struct udevice *dev) static int mxc_gpio_ofdata_to_platdata(struct udevice *dev) { - fdt_addr_t addr; struct mxc_gpio_plat *plat = dev_get_platdata(dev); + if (!CONFIG_IS_ENABLED(OF_PLATDATA)) { + fdt_addr_t addr; - addr = dev_read_addr(dev); - if (addr == FDT_ADDR_T_NONE) - return -EINVAL; + addr = dev_read_addr(dev); + if (addr == FDT_ADDR_T_NONE) + return -EINVAL; - plat->regs = (struct gpio_regs *)addr; + plat->regs = (struct gpio_regs *)addr; + } plat->bank_index = dev->req_seq; return 0; @@ -332,6 +346,8 @@ U_BOOT_DRIVER(gpio_mxc) = { .bind = mxc_gpio_bind, }; +U_BOOT_DRIVER_ALIAS(gpio_mxc, fsl_imx6q_gpio) + #if !CONFIG_IS_ENABLED(OF_CONTROL) static const struct mxc_gpio_plat mxc_plat[] = { { 0, (struct gpio_regs *)GPIO1_BASE_ADDR }, -- 2.20.1
[PATCH v3 1/6] mmc: fsl_esdhc_imx: rename driver name to match ll_entry
As discussed in commit rename fsl_esdhc_imx driver to allow the OF_PLATDATA support. Signed-off-by: Walter Lozano Reviewed-by: Simon Glass --- (no changes since v1) drivers/mmc/fsl_esdhc_imx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 644f4651fb..6b727dbbd5 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1644,7 +1644,7 @@ static int fsl_esdhc_bind(struct udevice *dev) #endif U_BOOT_DRIVER(fsl_esdhc) = { - .name = "fsl-esdhc-mmc", + .name = "fsl_esdhc", .id = UCLASS_MMC, .of_match = fsl_esdhc_ids, .ops= _esdhc_ops, -- 2.20.1
[PATCH v3 2/6] mmc: fsl_esdhc_imx: add OF_PLATDATA support
In order to reduce the footprint of SPL by removing dtb and library overhead, add OF_PLATDATA support to fsl_esdhc_imx. This initial approach does not support card detection, which will be enabled after adding OF_PLATDATA support to GPIO. Signed-off-by: Walter Lozano Reviewed-by: Simon Glass --- Changes in v3: - Fix commit message drivers/mmc/fsl_esdhc_imx.c | 67 + 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 6b727dbbd5..2713682cf7 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -33,6 +33,9 @@ #include #include #include +#include +#include +#include #if !CONFIG_IS_ENABLED(BLK) #include "mmc_private.h" @@ -102,6 +105,11 @@ struct fsl_esdhc { }; struct fsl_esdhc_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + /* Put this first since driver model will copy the data here */ + struct dtd_fsl_esdhc dtplat; +#endif + struct mmc_config cfg; struct mmc mmc; }; @@ -1378,25 +1386,19 @@ __weak void init_clk_usdhc(u32 index) { } -static int fsl_esdhc_probe(struct udevice *dev) +static int fsl_esdhc_ofdata_to_platdata(struct udevice *dev) { - struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); - struct fsl_esdhc_plat *plat = dev_get_platdata(dev); +#if !CONFIG_IS_ENABLED(OF_PLATDATA) struct fsl_esdhc_priv *priv = dev_get_priv(dev); - const void *fdt = gd->fdt_blob; - int node = dev_of_offset(dev); - struct esdhc_soc_data *data = - (struct esdhc_soc_data *)dev_get_driver_data(dev); #if CONFIG_IS_ENABLED(DM_REGULATOR) struct udevice *vqmmc_dev; + int ret; #endif + const void *fdt = gd->fdt_blob; + int node = dev_of_offset(dev); + fdt_addr_t addr; unsigned int val; - struct mmc *mmc; -#if !CONFIG_IS_ENABLED(BLK) - struct blk_desc *bdesc; -#endif - int ret; addr = dev_read_addr(dev); if (addr == FDT_ADDR_T_NONE) @@ -1404,8 +1406,6 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->esdhc_regs = (struct fsl_esdhc *)addr; priv->dev = dev; priv->mode = -1; - if (data) - priv->flags = data->flags; val = dev_read_u32_default(dev, "bus-width", -1); if (val == 8) @@ -1469,6 +1469,40 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->vs18_enable = 1; } #endif +#endif + return 0; +} + +static int fsl_esdhc_probe(struct udevice *dev) +{ + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); + struct fsl_esdhc_plat *plat = dev_get_platdata(dev); + struct fsl_esdhc_priv *priv = dev_get_priv(dev); + struct esdhc_soc_data *data = + (struct esdhc_soc_data *)dev_get_driver_data(dev); + struct mmc *mmc; +#if !CONFIG_IS_ENABLED(BLK) + struct blk_desc *bdesc; +#endif + int ret; + +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dtd_fsl_esdhc *dtplat = >dtplat; + unsigned int val; + + priv->esdhc_regs = map_sysmem(dtplat->reg[0], dtplat->reg[1]); + val = plat->dtplat.bus_width; + if (val == 8) + priv->bus_width = 8; + else if (val == 4) + priv->bus_width = 4; + else + priv->bus_width = 1; + priv->non_removable = 1; +#endif + + if (data) + priv->flags = data->flags; /* * TODO: @@ -1520,9 +1554,11 @@ static int fsl_esdhc_probe(struct udevice *dev) return ret; } +#if !CONFIG_IS_ENABLED(OF_PLATDATA) ret = mmc_of_parse(dev, >cfg); if (ret) return ret; +#endif mmc = >mmc; mmc->cfg = >cfg; @@ -1647,6 +1683,7 @@ U_BOOT_DRIVER(fsl_esdhc) = { .name = "fsl_esdhc", .id = UCLASS_MMC, .of_match = fsl_esdhc_ids, + .ofdata_to_platdata = fsl_esdhc_ofdata_to_platdata, .ops= _esdhc_ops, #if CONFIG_IS_ENABLED(BLK) .bind = fsl_esdhc_bind, @@ -1655,4 +1692,6 @@ U_BOOT_DRIVER(fsl_esdhc) = { .platdata_auto_alloc_size = sizeof(struct fsl_esdhc_plat), .priv_auto_alloc_size = sizeof(struct fsl_esdhc_priv), }; + +U_BOOT_DRIVER_ALIAS(fsl_esdhc, fsl_imx6q_usdhc) #endif -- 2.20.1
[PATCH v3 0/6] mx6cuboxi: enable OF_PLATDATA with MMC support
The SPL in iMX6 boards is restricted to 68 KB as this is the free available space in OCRAM for most revisions. In this context, adding OF_CONTROL and DM increases the SPL size which could make it difficult to add specific features required for custom scenarios. These patches aim to take advantage of OF_PLATADATA in order to reduce the SPL size in this scenario, by parsing DT data to generate platdata structures, and thus removing the overhead caused by DT and related libraries. This series is focused in MMC driver, which is used for boot in boards such as Cubox-i. Also, in order to support CD, the OF_PLATDATA support is also implemented on GPIO driver. After applying this setting the SPL reduction is 5 KB, which partially compensates the increment due to DM. Changes in v3: - Fix commit message - Rework to resolve merge conflicts - Change #ifdef to if when possible Changes in v2: - Improve commit message with footprint reduction Walter Lozano (6): mmc: fsl_esdhc_imx: rename driver name to match ll_entry mmc: fsl_esdhc_imx: add OF_PLATDATA support gpio: mxc_gpio: add OF_PLATDATA support mmc: fsl_esdhc_imx: add CD support when OF_PLATDATA is enabled drivers: rename more drivers to match compatible string mx6cuboxi: enable OF_PLATDATA configs/mx6cuboxi_defconfig| 1 + drivers/gpio/mxc_gpio.c| 26 +++-- drivers/mmc/fsl_esdhc_imx.c| 92 +- drivers/pinctrl/nxp/pinctrl-imx6.c | 6 +- drivers/video/imx/mxc_ipuv3_fb.c | 4 +- 5 files changed, 105 insertions(+), 24 deletions(-) -- 2.20.1
[PATCH v2] dtoc: add coverage test for unicode error
Add an additional test to dtoc in order improve the coverage, specifically to take into account the case of unicode error when scanning drivers. Signed-off-by: Walter Lozano --- Changes in v2: - Add missing files - Extend scan_drivers to use both relative and absolute paths tools/dtoc/dtb_platdata.py| 18 +++--- tools/dtoc/dtoc_test_scan_drivers.cxx | 1 + tools/dtoc/test_dtoc.py | 26 ++ 3 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 tools/dtoc/dtoc_test_scan_drivers.cxx diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index c28768f4a2..f870ebdb78 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -145,8 +145,10 @@ class DtbPlatdata(object): U_BOOT_DRIVER_ALIAS(driver_alias, driver_name) value: Driver name declared with U_BOOT_DRIVER(driver_name) _links: List of links to be included in dm_populate_phandle_data() +_drivers_additional: List of additional drivers to use during scanning """ -def __init__(self, dtb_fname, include_disabled, warning_disabled): +def __init__(self, dtb_fname, include_disabled, warning_disabled, + drivers_additional=[]): self._fdt = None self._dtb_fname = dtb_fname self._valid_nodes = None @@ -157,6 +159,7 @@ class DtbPlatdata(object): self._drivers = [] self._driver_aliases = {} self._links = [] +self._drivers_additional = drivers_additional def get_normalized_compat_name(self, node): """Get a node's normalized compat name @@ -338,6 +341,14 @@ class DtbPlatdata(object): continue self.scan_driver(dirpath + '/' + fn) +for fn in self._drivers_additional: +if not isinstance(fn, str) or len(fn) == 0: +continue +if fn[0] == '/': +self.scan_driver(fn) +else: +self.scan_driver(basedir + '/' + fn) + def scan_dtb(self): """Scan the device tree to obtain a tree of nodes and properties @@ -654,7 +665,8 @@ class DtbPlatdata(object): self.out(''.join(self.get_buf())) -def run_steps(args, dtb_file, include_disabled, output, warning_disabled=False): +def run_steps(args, dtb_file, include_disabled, output, warning_disabled=False, + drivers_additional=[]): """Run all the steps of the dtoc tool Args: @@ -666,7 +678,7 @@ def run_steps(args, dtb_file, include_disabled, output, warning_disabled=False): if not args: raise ValueError('Please specify a command: struct, platdata') -plat = DtbPlatdata(dtb_file, include_disabled, warning_disabled) +plat = DtbPlatdata(dtb_file, include_disabled, warning_disabled, drivers_additional) plat.scan_drivers() plat.scan_dtb() plat.scan_tree() diff --git a/tools/dtoc/dtoc_test_scan_drivers.cxx b/tools/dtoc/dtoc_test_scan_drivers.cxx new file mode 100644 index 00..557c692ba2 --- /dev/null +++ b/tools/dtoc/dtoc_test_scan_drivers.cxx @@ -0,0 +1 @@ +U_BOOT_DRIVER_ALIAS(sandbox_gpio, sandbox_gpio_alias2) diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 169ecd6e6e..f1b0eb824a 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -13,6 +13,7 @@ import collections import os import struct import sys +import tempfile import unittest from dtoc import dtb_platdata @@ -829,3 +830,28 @@ U_BOOT_DEVICE(spl_test2) = { self.run_test(['invalid-cmd'], dtb_file, output) self.assertIn("Unknown command 'invalid-cmd': (use: struct, platdata)", str(e.exception)) + +def testScanDrivers(self): +"""Test running dtoc with additional drivers to scan""" +dtb_file = get_dtb_file('dtoc_test_simple.dts') +output = tools.GetOutputFilename('output') +with test_util.capture_sys_output() as (stdout, stderr): +dtb_platdata.run_steps(['struct'], dtb_file, False, output, True, + [None, '', 'tools/dtoc/dtoc_test_scan_drivers.cxx']) + +def testUnicodeError(self): +"""Test running dtoc with an invalid unicode file + +To be able to perform this test without adding a weird text file which +would produce issues when using checkpatch.pl or patman, generate the +file at runtime and then process it. +""" +dtb_file = get_dtb_file('dtoc_test_simple.dts') +output = tools.GetOutputFilename('output') +driver_fn = '/tmp/' + next(tempfile._get_candidate_names()) +with open(driver_fn, 'wb+') as df: +df.write(b'\x81') + +with test_util.capture_sys_output() as (stdout, stderr): +dtb_platdata.run_steps(['struct'], dtb_file, False, output, True, + [driver_fn]) -- 2.20.1
Re: [PATCH] dtoc: add coverage test for unicode error
Hi Simon, On 27/7/20 20:35, Simon Glass wrote: Hi Walter, On Mon, 20 Jul 2020 at 12:29, Walter Lozano wrote: Add an additional test to dtoc in order improve the coverage, specifically to take into account the case of unicode error when scanning drivers. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 14 +++--- tools/dtoc/test_dtoc.py| 18 ++ 2 files changed, 29 insertions(+), 3 deletions(-) This seems to be missing a .cxx file, so gives a build error. Sorry for the error. I'm sending a new version with some additional improvements. Regards, Walter
Re: [RFC PATCH v2 0/3] RFC: tiny-dm: Proposal for using driver model in SPL
Hi Simon, On 10/7/20 01:12, Walter Lozano wrote: Hi Simon, On 2/7/20 18:10, Simon Glass wrote: This series provides a proposed enhancement to driver model to reduce overhead in SPL. These patches should not be reviewed other than to comment on the approach. The code is all lumped together in a few patches and so cannot be applied as is. For now, the source tree is available at: https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/dtoc-working Comments welcome! Benefits (good news) As an example of the impact of tiny-dm, chromebook_jerry is converted to use it. This shows approximately a 30% reduction in code and data size and a 85% reduction in malloc() space over of-platdata: text data bss dec hex filename 25248 1836 12 27096 69d8 spl/u-boot-spl (original with DT) 19727 3436 12 23175 5a87 spl/u-boot-spl (OF_PLATDATA) 78% 187% 100% 86% as %age of original 13784 1408 12 15204 3b64 spl/u-boot-spl (SPL_TINY) 70% 41% 100% 66% as %age of platdata 55% 77% 100% 56% as %age of original SPL malloc() usage drops from 944 bytes (OF_PLATDATA) to 116 (SPL_TINY). Overall the 'overhead' of tiny-dm is much less than the full driver model. Code size is currently about 600 bytes for these functions on Thumb2: 0054 T tiny_dev_probe 0034 T tiny_dev_get_by_drvdata 0024 T tiny_dev_find 001a T tiny_dev_get 003c T tinydev_alloc_data 002a t tinydev_lookup_data 0022 T tinydev_ensure_data 0014 T tinydev_get_data 0004 T tinydev_get_parent Effort (bad news) - Unfortunately it is quite a bit of work to convert drivers over to tiny-dm. First, the of-platdata conversion must be done. But on top of that, tiny-dm needs entirely separate code for dealing with devices. This means that instead of 'struct udevice' and 'struct uclass' there is just 'struct tinydev'. Each driver and uclass must be modified to support both, pulling common code into internal static functions. Another option -- Note: It is assumed that any board that is space-contrained should use of-platdata in SPL (see doc/driver-model/of-plat.rst). This is shown to reduce device-tree overhead by approximately 4KB. Designing tiny-dm has suggested a number of things that could be changed in the current driver model to make it more space-efficient for TPL and SPL. The ones with least impact on driver code are (CS=reduces code size, DS=reduces data size): CS - drop driver_bind() and create devices (struct udevice) at build-time CS - allocate all device- and uclass-private data at build-time CS - remove all but the basic operations for each uclass (e.g. SPI flash only supports reading) DS - use 8-bit indexes instead of 32/64-bit pointers for device pointers possible since these are created at build-time) DS - use singly-linked lists DS - use 16-bit offsets to private data, instead of 32/64-bit pointers (possible since it is all in SRAM relative to malloc() base, presumably word-aligned and < 256KB) DS - move private pointers into a separate data structure so that NULLs are not stored CS / DS - Combine req_seq and seq and calculate the new value at build-time More difficult are: DS - drop some of the lesser-used driver and uclass methods DS - drop all uclass methods except init() DS - drop all driver methods except probe() CS / DS - drop uclasses and require drivers to manually call uclass functions Even with all of this we would not reach tiny-dm and it would muddy up the driver-model datas structures. But we might come close to tiny-dm on size and there are some advantages: - much of the benefit would apply to all boards that use of-platdata (i.e. with very little effort on behalf of board maintainers) - the impact on the code base is much less (we keep a single, unified driver mode in SPL and U-Boot proper) Overall I think it is worth looking at this option. While it doesn't have the 'nuclear' impact of tiny-dm, neither does it mess with the U-Boot driver code as much and it is easier to learn. Thanks for your hard work on this topic. I think that there is great value in this research and in this conclusion. It is clear that there two different approaches, but I feel that the improvement to the current DM implementation would have a higher impact in the community. Since the first version of this proposal I have been thinking in a solution that takes some of the advantages of tiny-dm idea but that does not require so much effort. This seems to be aligned with what you have been explaining in this section. I found interesting your proposal about simplification some data structu
Re: [RFC 3/4] dtoc: add support for generate stuct udevice_id
Hi Simon, On 26/7/20 11:53, Simon Glass wrote: Hi Walter, On Tue, 7 Jul 2020 at 08:08, Walter Lozano wrote: Hi Simon On 6/7/20 16:21, Simon Glass wrote: Hi Walter, On Fri, 19 Jun 2020 at 15:12, Walter Lozano wrote: Based on several reports there is an increasing concern in the impact of adding additional features to drivers based on compatible strings. A good example of this situation is found in [1]. In order to reduce this impact and as an initial step for further reduction, propose a new way to declare compatible strings, which allows to only include the useful ones. What are the useful ones? The useful ones would be those that are used by the selected DTB by the current configuration. The idea of this patch is to declare all the possible compatible strings in a way that dtoc can generate code for only those which are going to be used, and in this way avoid lots of #ifdef like the ones shows in http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ The idea is to define compatible strings in a way to be easily parsed by dtoc, which will be responsible to build struct udevice_id [] based on the compatible strings present in the dtb. Additional features can be easily added, such as define constants depending on the presence of compatible strings, which allows to enable code blocks only in such cases without the need of adding additional configuration options. [1] http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 32 1 file changed, 32 insertions(+) I think dtoc should be able to parse the compatible strings as they are today - e.g. see the tiny-dm stuff. Yes, I agree. My idea is that dtoc parses compatible strings as they are today but also in this new way. The reason for this is to allow dtoc to generate the code to include the useful compatible strings. Of course, this only makes sense if the idea of generating the compatible string associated code is accepted. What do you think? I think this is useful and better than using #ifdef in the source code for this sort of thing. We need a way to specify the driver_data value as well, right? Yes, I agree, it is better than #ifdef and c/ould give us some extra functionality. What doe you mean by driver_data value? Are you referring to the data field? like static struct esdhc_soc_data usdhc_imx7d_data = { .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200 | ESDHC_FLAG_HS400, }; If that is the case, I was thinking in defining a constant when specific compatible strings are enabled by dtoc, based in the above case #ifdef FSL_ESDHC_IMX_V2 static struct esdhc_soc_data usdhc_imx7d_data = { .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200 | ESDHC_FLAG_HS400, }; #endif COMPATIBLE(FSL_ESDHC, "fsl,imx7d-usdhc", _imx7d_data, FSL_ESDHC_IMX_V2) So when dtoc parses COMPATIBLE and determines that compatible "fsl,imx7d-usdhc" should be added it also defines FSL_ESDHC_IMX_V2. This is alsoAs I comment you in the tread about tiny-dm I think that we can save some space following your suggestions, and for instance implement Re naming, perhaps DT_COMPAT() might be better than COMPATIBLE()? Or even a name that indicates that it is optional, like DT_OPT_COMPAT() ? I totally agree, naming is very important, and DT_COMPAT() is much better. What I don't fully understand is what are the cases for DT_OPT_COMPAT(), could you please clarify? Regards, Walter
[PATCH v2 2/3] drivers: avoid using aliases on drivers when OF_PLATDATA is enabled
After latest improvements on OF_PLATDATA struct names are generated based on driver name instead of compatible strings. With this in mind, using aliases in drivers are not longer needed. This patch removes code that tried to handle these kind of aliases to improve readability. Signed-off-by: Walter Lozano --- Changes in v2: - Remove aliases from drivers drivers/gpio/mxs_gpio.c | 10 ++ drivers/mmc/mxsmmc.c| 10 ++ drivers/spi/mxs_spi.c | 10 ++ 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c index cb797261b7..b30adf50da 100644 --- a/drivers/gpio/mxs_gpio.c +++ b/drivers/gpio/mxs_gpio.c @@ -138,12 +138,6 @@ int name_to_gpio(const char *name) #include #define MXS_MAX_GPIO_PER_BANK 32 -#ifdef CONFIG_MX28 -#define dtd_fsl_imx_gpio dtd_fsl_imx28_gpio -#else /* CONFIG_MX23 */ -#define dtd_fsl_imx_gpio dtd_fsl_imx23_gpio -#endif - DECLARE_GLOBAL_DATA_PTR; /* * According to i.MX28 Reference Manual: @@ -158,7 +152,7 @@ DECLARE_GLOBAL_DATA_PTR; struct mxs_gpio_platdata { #if CONFIG_IS_ENABLED(OF_PLATDATA) - struct dtd_fsl_imx_gpio dtplat; + struct dtd_fsl_imx23_gpio dtplat; #endif unsigned int bank; int gpio_ranges; @@ -247,7 +241,7 @@ static int mxs_gpio_probe(struct udevice *dev) char name[16], *str; #if CONFIG_IS_ENABLED(OF_PLATDATA) - struct dtd_fsl_imx_gpio *dtplat = >dtplat; + struct dtd_fsl_imx23_gpio *dtplat = >dtplat; priv->bank = (unsigned int)dtplat->reg[0]; uc_priv->gpio_count = dtplat->gpio_ranges[3]; #else diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c index c6a06b9ca8..c368271c23 100644 --- a/drivers/mmc/mxsmmc.c +++ b/drivers/mmc/mxsmmc.c @@ -52,15 +52,9 @@ struct mxsmmc_priv { #include #include -#ifdef CONFIG_MX28 -#define dtd_fsl_imx_mmc dtd_fsl_imx28_mmc -#else /* CONFIG_MX23 */ -#define dtd_fsl_imx_mmc dtd_fsl_imx23_mmc -#endif - struct mxsmmc_platdata { #if CONFIG_IS_ENABLED(OF_PLATDATA) - struct dtd_fsl_imx_mmc dtplat; + struct dtd_fsl_imx23_mmc dtplat; #endif struct mmc_config cfg; struct mmc mmc; @@ -581,7 +575,7 @@ static int mxsmmc_probe(struct udevice *dev) debug("%s: probe\n", __func__); #if CONFIG_IS_ENABLED(OF_PLATDATA) - struct dtd_fsl_imx_mmc *dtplat = >dtplat; + struct dtd_fsl_imx23_mmc *dtplat = >dtplat; struct phandle_1_arg *p1a = >clocks[0]; priv->buswidth = dtplat->bus_width; diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c index 3c1af839c0..fb0af02be0 100644 --- a/drivers/spi/mxs_spi.c +++ b/drivers/spi/mxs_spi.c @@ -41,15 +41,9 @@ #define MXS_SSP_IMX23_CLKID_SSP0 33 #define MXS_SSP_IMX28_CLKID_SSP0 46 -#ifdef CONFIG_MX28 -#define dtd_fsl_imx_spi dtd_fsl_imx28_spi -#else /* CONFIG_MX23 */ -#define dtd_fsl_imx_spi dtd_fsl_imx23_spi -#endif - struct mxs_spi_platdata { #if CONFIG_IS_ENABLED(OF_PLATDATA) - struct dtd_fsl_imx_spi dtplat; + struct dtd_fsl_imx23_spi dtplat; #endif s32 frequency; /* Default clock frequency, -1 for none */ fdt_addr_t base;/* SPI IP block base address */ @@ -324,7 +318,7 @@ static int mxs_spi_probe(struct udevice *bus) debug("%s: probe\n", __func__); #if CONFIG_IS_ENABLED(OF_PLATDATA) - struct dtd_fsl_imx_spi *dtplat = >dtplat; + struct dtd_fsl_imx23_spi *dtplat = >dtplat; struct phandle_1_arg *p1a = >clocks[0]; priv->regs = (struct mxs_ssp_regs *)dtplat->reg[0]; -- 2.20.1
[PATCH v2 1/3] dtoc: look for compatible string aliases in driver list
Currently dtoc checks if the first compatible string in a dtb node matches either a driver o driver alias name, without taking into account any other compatible string in the list. In the case that no driver matches the first compatible string a warning is printed and the U_BOOT_DEVICE is not being declared correctly. This patch adds dtoc's support for try all the compatible strings in the dtb node, in an effort to find the correct driver. Signed-off-by: Walter Lozano --- (no changes since v1) tools/dtoc/dtb_platdata.py | 45 tools/dtoc/dtoc_test_aliases.dts | 5 tools/dtoc/test_dtoc.py | 20 +++--- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index c148c49625..b1b082e508 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -111,21 +111,17 @@ def get_value(ftype, value): return '%#x' % value def get_compat_name(node): -"""Get a node's first compatible string as a C identifier +"""Get the node's list of compatible string as a C identifiers Args: node: Node object to check Return: -Tuple: -C identifier for the first compatible string -List of C identifiers for all the other compatible strings -(possibly empty) +List of C identifiers for all the compatible strings """ compat = node.props['compatible'].value -aliases = [] -if isinstance(compat, list): -compat, aliases = compat[0], compat[1:] -return conv_name_to_c(compat), [conv_name_to_c(a) for a in aliases] +if not isinstance(compat, list): +compat = [compat] +return [conv_name_to_c(c) for c in compat] class DtbPlatdata(object): @@ -169,7 +165,7 @@ class DtbPlatdata(object): def get_normalized_compat_name(self, node): """Get a node's normalized compat name -Returns a valid driver name by retrieving node's first compatible +Returns a valid driver name by retrieving node's list of compatible string as a C identifier and performing a check against _drivers and a lookup in driver_aliases printing a warning in case of failure. @@ -183,19 +179,24 @@ class DtbPlatdata(object): In case of no match found, the return will be the same as get_compat_name() """ -compat_c, aliases_c = get_compat_name(node) -if compat_c not in self._drivers: -compat_c_old = compat_c -compat_c = self._driver_aliases.get(compat_c) -if not compat_c: -if not self._warning_disabled: -print('WARNING: the driver %s was not found in the driver list' - % (compat_c_old)) -compat_c = compat_c_old -else: -aliases_c = [compat_c_old] + aliases_c +compat_list_c = get_compat_name(node) + +for compat_c in compat_list_c: +if not compat_c in self._drivers: +compat_c = self._driver_aliases.get(compat_c) +if not compat_c: +continue + +aliases_c = compat_list_c +if compat_c in aliases_c: +aliases_c.remove(compat_c) +return compat_c, aliases_c + +if not self._warning_disabled: +print('WARNING: the driver %s was not found in the driver list' + % (compat_list_c[0])) -return compat_c, aliases_c +return compat_list_c[0], compat_list_c[1:] def setup_output(self, fname): """Set up the output destination diff --git a/tools/dtoc/dtoc_test_aliases.dts b/tools/dtoc/dtoc_test_aliases.dts index e545816f4e..ae33716863 100644 --- a/tools/dtoc/dtoc_test_aliases.dts +++ b/tools/dtoc/dtoc_test_aliases.dts @@ -14,4 +14,9 @@ intval = <1>; }; + spl-test2 { + u-boot,dm-pre-reloc; + compatible = "compat1", "simple_bus"; + intval = <1>; + }; }; diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 3c8e343b1f..edb3912e94 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -144,18 +144,18 @@ class TestDtoc(unittest.TestCase): prop = Prop(['rockchip,rk3399-sdhci-5.1', 'arasan,sdhci-5.1']) node = Node({'compatible': prop}) -self.assertEqual(('rockchip_rk3399_sdhci_5_1', ['arasan_sdhci_5_1']), +self.assertEqual((['rockchip_rk3399_sdhci_5_1', 'arasan_sdhci_5_1']), get_compat_name(node)) prop = Prop(['rockchip,rk3399-sdhci-5.1']) node = Node({'compatible': prop}) -self.assertEqual(('rockchip_rk3399_sdhci_5_1', []), +self.assertEqual((['rockchip_rk3399_s
[PATCH v2 3/3] dtoc: remove compatible string aliases support
After latest improvements in dtoc, compatible strings are checked against driver and driver alias list to get a valid driver name. With this new feature the list of compatible string aliases seems not useful any more. Signed-off-by: Walter Lozano --- (no changes since v1) tools/dtoc/dtb_platdata.py | 13 tools/dtoc/test_dtoc.py| 43 -- 2 files changed, 56 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index b1b082e508..c28768f4a2 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -139,9 +139,6 @@ class DtbPlatdata(object): _outfile: The current output file (sys.stdout or a real file) _warning_disabled: true to disable warnings about driver names not found _lines: Stashed list of output lines for outputting in the future -_aliases: Dict that hold aliases for compatible strings -key: First compatible string declared in a node -value: List of additional compatible strings declared in a node _drivers: List of valid driver names found in drivers/ _driver_aliases: Dict that holds aliases for driver names key: Driver alias declared with @@ -157,7 +154,6 @@ class DtbPlatdata(object): self._outfile = None self._warning_disabled = warning_disabled self._lines = [] -self._aliases = {} self._drivers = [] self._driver_aliases = {} self._links = [] @@ -483,10 +479,6 @@ class DtbPlatdata(object): prop.Widen(struct[name]) upto += 1 -struct_name, aliases = self.get_normalized_compat_name(node) -for alias in aliases: -self._aliases[alias] = struct_name - return structs def scan_phandles(self): @@ -549,11 +541,6 @@ class DtbPlatdata(object): self.out(';\n') self.out('};\n') -for alias, struct_name in self._aliases.items(): -if alias not in sorted(structs): -self.out('#define %s%s %s%s\n'% (STRUCT_PREFIX, alias, - STRUCT_PREFIX, struct_name)) - def output_node(self, node): """Output the C code for a node diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index edb3912e94..169ecd6e6e 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -290,7 +290,6 @@ struct dtd_sandbox_gpio { \tbool\t\tgpio_controller; \tfdt32_t\t\tsandbox_gpio_count; }; -#define dtd_sandbox_gpio_alias dtd_sandbox_gpio ''', data) self.run_test(['platdata'], dtb_file, output) @@ -555,48 +554,6 @@ void dm_populate_phandle_data(void) { self.assertIn("Node 'phandle-target' has no cells property", str(e.exception)) -def test_aliases(self): -"""Test output from a node with multiple compatible strings""" -dtb_file = get_dtb_file('dtoc_test_aliases.dts') -output = tools.GetOutputFilename('output') -self.run_test(['struct'], dtb_file, output) -with open(output) as infile: -data = infile.read() -self._CheckStrings(HEADER + ''' -struct dtd_compat1 { -\tfdt32_t\t\tintval; -}; -struct dtd_simple_bus { -\tfdt32_t\t\tintval; -}; -#define dtd_compat2_1_fred dtd_compat1 -#define dtd_compat3 dtd_compat1 -''', data) - -self.run_test(['platdata'], dtb_file, output) -with open(output) as infile: -data = infile.read() -self._CheckStrings(C_HEADER + ''' -static struct dtd_compat1 dtv_spl_test = { -\t.intval\t\t\t= 0x1, -}; -U_BOOT_DEVICE(spl_test) = { -\t.name\t\t= "compat1", -\t.platdata\t= _spl_test, -\t.platdata_size\t= sizeof(dtv_spl_test), -}; - -static struct dtd_simple_bus dtv_spl_test2 = { -\t.intval\t\t\t= 0x1, -}; -U_BOOT_DEVICE(spl_test2) = { -\t.name\t\t= "simple_bus", -\t.platdata\t= _spl_test2, -\t.platdata_size\t= sizeof(dtv_spl_test2), -}; - -''' + C_EMPTY_POPULATE_PHANDLE_DATA, data) - def test_addresses64(self): """Test output from a node with a 'reg' property with na=2, ns=2""" dtb_file = get_dtb_file('dtoc_test_addr64.dts') -- 2.20.1
[PATCH v2 0/3] dtoc: improve compatible string aliases support
This series adds additional improvements to dtoc, which are focused in compatible string aliases. First, enhance dtoc to use not only the first but all the compatible strings in a dtb node in order to find a valid driver name. Second, remove the aliases support from some drivers as it is not longer needed. Last, remove compatible string aliases from dtoc which were used to generate mappings between struct names. This is not longer needed since now a valid driver name is used. Changes in v2: - Remove aliases from drivers Walter Lozano (3): dtoc: look for compatible string aliases in driver list drivers: avoid using aliases on drivers when OF_PLATDATA is enabled dtoc: remove compatible string aliases support drivers/gpio/mxs_gpio.c | 10 ++ drivers/mmc/mxsmmc.c | 10 ++ drivers/spi/mxs_spi.c| 10 ++ tools/dtoc/dtb_platdata.py | 58 +--- tools/dtoc/dtoc_test_aliases.dts | 5 +++ tools/dtoc/test_dtoc.py | 39 +++-- 6 files changed, 38 insertions(+), 94 deletions(-) -- 2.20.1
Re: [PATCH v2 0/6] mx6cuboxi: enable OF_PLATDATA with MMC support
Hi Peng On 22/7/20 22:34, Peng Fan wrote: Hi Walter, Subject: [PATCH v2 0/6] mx6cuboxi: enable OF_PLATDATA with MMC support The SPL in iMX6 boards is restricted to 68 KB as this is the free available space in OCRAM for most revisions. In this context, adding OF_CONTROL and DM increases the SPL size which could make it difficult to add specific features required for custom scenarios. These patches aim to take advantage of OF_PLATADATA in order to reduce the SPL size in this scenario, by parsing DT data to generate platdata structures, and thus removing the overhead caused by DT and related libraries. This series is focused in MMC driver, which is used for boot in boards such as Cubox-i. Also, in order to support CD, the OF_PLATDATA support is also implemented on GPIO driver. Do you have some data that how much is decreased using OF_PLATDATA? Yes, I updated the commit message on Patch 6/6, but should have updated the cover letter. With OF_PLATDATA enabled the reduction on mx6cuboxi_defconfig is 5 KB, which is half of the overhead introduced by DM on this config. Additionally, I've in my backlog some additional patches for further improvements. Regards, Walter Changes in v2: - Improve commit message with footprint reduction Walter Lozano (6): mmc: fsl_esdhc_imx: rename driver name to match ll_entry mmc: fsl_esdhc_imx: add OF_PLATDATA support gpio: mxc_gpio: add OF_PLATDATA support mmc: fsl_esdhc_imx: add CD support when OF_PLATDATA is enabled drivers: rename more drivers to match compatible string mx6cuboxi: enable OF_PLATDATA configs/mx6cuboxi_defconfig| 1 + drivers/gpio/mxc_gpio.c| 18 +- drivers/mmc/fsl_esdhc_imx.c| 94 +- drivers/pinctrl/nxp/pinctrl-imx6.c | 6 +- drivers/video/imx/mxc_ipuv3_fb.c | 4 +- 5 files changed, 103 insertions(+), 20 deletions(-) -- 2.20.1
[PATCH v2 5/6] drivers: rename more drivers to match compatible string
Continuing with the approach in commit rename additional drivers to allow the OF_PLATDATA support. Signed-off-by: Walter Lozano --- (no changes since v1) drivers/pinctrl/nxp/pinctrl-imx6.c | 6 -- drivers/video/imx/mxc_ipuv3_fb.c | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c index aafa3057ad..84004e5921 100644 --- a/drivers/pinctrl/nxp/pinctrl-imx6.c +++ b/drivers/pinctrl/nxp/pinctrl-imx6.c @@ -41,8 +41,8 @@ static const struct udevice_id imx6_pinctrl_match[] = { { /* sentinel */ } }; -U_BOOT_DRIVER(imx6_pinctrl) = { - .name = "imx6-pinctrl", +U_BOOT_DRIVER(fsl_imx6q_iomuxc) = { + .name = "fsl_imx6q_iomuxc", .id = UCLASS_PINCTRL, .of_match = of_match_ptr(imx6_pinctrl_match), .probe = imx6_pinctrl_probe, @@ -51,3 +51,5 @@ U_BOOT_DRIVER(imx6_pinctrl) = { .ops = _pinctrl_ops, .flags = DM_FLAG_PRE_RELOC, }; + +U_BOOT_DRIVER_ALIAS(fsl_imx6q_iomuxc, fsl_imx6dl_iomuxc) diff --git a/drivers/video/imx/mxc_ipuv3_fb.c b/drivers/video/imx/mxc_ipuv3_fb.c index 587d62f2d8..492bc3e829 100644 --- a/drivers/video/imx/mxc_ipuv3_fb.c +++ b/drivers/video/imx/mxc_ipuv3_fb.c @@ -660,8 +660,8 @@ static const struct udevice_id ipuv3_video_ids[] = { { } }; -U_BOOT_DRIVER(ipuv3_video) = { - .name = "ipuv3_video", +U_BOOT_DRIVER(fsl_imx6q_ipu) = { + .name = "fsl_imx6q_ipu", .id = UCLASS_VIDEO, .of_match = ipuv3_video_ids, .bind = ipuv3_video_bind, -- 2.20.1
[PATCH v2 2/6] mmc: fsl_esdhc_imx: add OF_PLATDATA support
In order to reduce the footprint of SPL by removing dtb and library overhead add OF_PLATDATA support to fsl_esdhc_imx. This initial approach does not support card detection, which will be enabled after adding OF_PLATDATA support to GPIO. Signed-off-by: Walter Lozano --- (no changes since v1) drivers/mmc/fsl_esdhc_imx.c | 67 + 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 7d76b144a7..f2509b5cc9 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -33,6 +33,9 @@ #include #include #include +#include +#include +#include #if !CONFIG_IS_ENABLED(BLK) #include "mmc_private.h" @@ -102,6 +105,11 @@ struct fsl_esdhc { }; struct fsl_esdhc_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + /* Put this first since driver model will copy the data here */ + struct dtd_fsl_esdhc dtplat; +#endif + struct mmc_config cfg; struct mmc mmc; }; @@ -1378,25 +1386,19 @@ __weak void init_clk_usdhc(u32 index) { } -static int fsl_esdhc_probe(struct udevice *dev) +static int fsl_esdhc_ofdata_to_platdata(struct udevice *dev) { - struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); - struct fsl_esdhc_plat *plat = dev_get_platdata(dev); +#if !CONFIG_IS_ENABLED(OF_PLATDATA) struct fsl_esdhc_priv *priv = dev_get_priv(dev); - const void *fdt = gd->fdt_blob; - int node = dev_of_offset(dev); - struct esdhc_soc_data *data = - (struct esdhc_soc_data *)dev_get_driver_data(dev); #if CONFIG_IS_ENABLED(DM_REGULATOR) struct udevice *vqmmc_dev; + int ret; #endif + const void *fdt = gd->fdt_blob; + int node = dev_of_offset(dev); + fdt_addr_t addr; unsigned int val; - struct mmc *mmc; -#if !CONFIG_IS_ENABLED(BLK) - struct blk_desc *bdesc; -#endif - int ret; addr = dev_read_addr(dev); if (addr == FDT_ADDR_T_NONE) @@ -1404,8 +1406,6 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->esdhc_regs = (struct fsl_esdhc *)addr; priv->dev = dev; priv->mode = -1; - if (data) - priv->flags = data->flags; val = dev_read_u32_default(dev, "bus-width", -1); if (val == 8) @@ -1469,6 +1469,40 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->vs18_enable = 1; } #endif +#endif + return 0; +} + +static int fsl_esdhc_probe(struct udevice *dev) +{ + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); + struct fsl_esdhc_plat *plat = dev_get_platdata(dev); + struct fsl_esdhc_priv *priv = dev_get_priv(dev); + struct esdhc_soc_data *data = + (struct esdhc_soc_data *)dev_get_driver_data(dev); + struct mmc *mmc; +#if !CONFIG_IS_ENABLED(BLK) + struct blk_desc *bdesc; +#endif + int ret; + +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dtd_fsl_esdhc *dtplat = >dtplat; + unsigned int val; + + priv->esdhc_regs = map_sysmem(dtplat->reg[0], dtplat->reg[1]); + val = plat->dtplat.bus_width; + if (val == 8) + priv->bus_width = 8; + else if (val == 4) + priv->bus_width = 4; + else + priv->bus_width = 1; + priv->non_removable = 1; +#endif + + if (data) + priv->flags = data->flags; /* * TODO: @@ -1520,9 +1554,11 @@ static int fsl_esdhc_probe(struct udevice *dev) return ret; } +#if !CONFIG_IS_ENABLED(OF_PLATDATA) ret = mmc_of_parse(dev, >cfg); if (ret) return ret; +#endif mmc = >mmc; mmc->cfg = >cfg; @@ -1647,6 +1683,7 @@ U_BOOT_DRIVER(fsl_esdhc) = { .name = "fsl_esdhc", .id = UCLASS_MMC, .of_match = fsl_esdhc_ids, + .ofdata_to_platdata = fsl_esdhc_ofdata_to_platdata, .ops= _esdhc_ops, #if CONFIG_IS_ENABLED(BLK) .bind = fsl_esdhc_bind, @@ -1655,4 +1692,6 @@ U_BOOT_DRIVER(fsl_esdhc) = { .platdata_auto_alloc_size = sizeof(struct fsl_esdhc_plat), .priv_auto_alloc_size = sizeof(struct fsl_esdhc_priv), }; + +U_BOOT_DRIVER_ALIAS(fsl_esdhc, fsl_imx6q_usdhc) #endif -- 2.20.1
[PATCH v2 1/6] mmc: fsl_esdhc_imx: rename driver name to match ll_entry
As discussed in commit rename fsl_esdhc_imx driver to allow the OF_PLATDATA support. Signed-off-by: Walter Lozano --- (no changes since v1) drivers/mmc/fsl_esdhc_imx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 5b61eeb214..7d76b144a7 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1644,7 +1644,7 @@ static int fsl_esdhc_bind(struct udevice *dev) #endif U_BOOT_DRIVER(fsl_esdhc) = { - .name = "fsl-esdhc-mmc", + .name = "fsl_esdhc", .id = UCLASS_MMC, .of_match = fsl_esdhc_ids, .ops= _esdhc_ops, -- 2.20.1
[PATCH v2 3/6] gpio: mxc_gpio: add OF_PLATDATA support
Continuing with the OF_PLATADATA support for iMX6 to reduce SPL footprint, add it to mxc_gpio. Thanks to this, it will be possible to enable card detection on MMC driver. Signed-off-by: Walter Lozano --- (no changes since v1) drivers/gpio/mxc_gpio.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 316dcc757b..fc49b5b577 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -13,6 +13,8 @@ #include #include #include +#include +#include enum mxc_gpio_direction { MXC_GPIO_DIRECTION_IN, @@ -22,6 +24,10 @@ enum mxc_gpio_direction { #define GPIO_PER_BANK 32 struct mxc_gpio_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + /* Put this first since driver model will copy the data here */ + struct dtd_gpio_mxc dtplat; +#endif int bank_index; struct gpio_regs *regs; }; @@ -306,8 +312,16 @@ static int mxc_gpio_bind(struct udevice *dev) * is statically initialized in U_BOOT_DEVICES.Here * will return. */ - if (plat) + + if (plat) { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dtd_gpio_mxc *dtplat = >dtplat; + + plat->regs = map_sysmem(dtplat->reg[0], dtplat->reg[1]); + plat->bank_index = dev->req_seq; +#endif return 0; + } addr = devfdt_get_addr(dev); if (addr == FDT_ADDR_T_NONE) @@ -350,6 +364,8 @@ U_BOOT_DRIVER(gpio_mxc) = { .bind = mxc_gpio_bind, }; +U_BOOT_DRIVER_ALIAS(gpio_mxc, fsl_imx6q_gpio) + #if !CONFIG_IS_ENABLED(OF_CONTROL) static const struct mxc_gpio_plat mxc_plat[] = { { 0, (struct gpio_regs *)GPIO1_BASE_ADDR }, -- 2.20.1
[PATCH v2 0/6] mx6cuboxi: enable OF_PLATDATA with MMC support
The SPL in iMX6 boards is restricted to 68 KB as this is the free available space in OCRAM for most revisions. In this context, adding OF_CONTROL and DM increases the SPL size which could make it difficult to add specific features required for custom scenarios. These patches aim to take advantage of OF_PLATADATA in order to reduce the SPL size in this scenario, by parsing DT data to generate platdata structures, and thus removing the overhead caused by DT and related libraries. This series is focused in MMC driver, which is used for boot in boards such as Cubox-i. Also, in order to support CD, the OF_PLATDATA support is also implemented on GPIO driver. Changes in v2: - Improve commit message with footprint reduction Walter Lozano (6): mmc: fsl_esdhc_imx: rename driver name to match ll_entry mmc: fsl_esdhc_imx: add OF_PLATDATA support gpio: mxc_gpio: add OF_PLATDATA support mmc: fsl_esdhc_imx: add CD support when OF_PLATDATA is enabled drivers: rename more drivers to match compatible string mx6cuboxi: enable OF_PLATDATA configs/mx6cuboxi_defconfig| 1 + drivers/gpio/mxc_gpio.c| 18 +- drivers/mmc/fsl_esdhc_imx.c| 94 +- drivers/pinctrl/nxp/pinctrl-imx6.c | 6 +- drivers/video/imx/mxc_ipuv3_fb.c | 4 +- 5 files changed, 103 insertions(+), 20 deletions(-) -- 2.20.1
Re: [PATCH 6/6] mx6cuboxi: enable OF_PLATDATA
Hi Fabio, On 21/7/20 17:58, Fabio Estevam wrote: Hi Walter, On Tue, Jul 21, 2020 at 2:24 PM Walter Lozano wrote: As both MMC and GPIO driver now supports OF_PLATDATA, enable it in defconfig in order to reduce the SPL footprint. Could you add in the commit log how many kB of SPL size reduction this gives? Sure, thanks for the feedback. Regards, Walter
[PATCH v2 6/6] mx6cuboxi: enable OF_PLATDATA
As both MMC and GPIO driver now supports OF_PLATDATA, enable it in defconfig in order to reduce the SPL footprint. After applying this setting the SPL reduction is 5 KB, which partially compensates the increment due to DM. Signed-off-by: Walter Lozano --- Changes in v2: - Improve commit message with footprint reduction configs/mx6cuboxi_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig index cd4d4da43e..54e5b76f3c 100644 --- a/configs/mx6cuboxi_defconfig +++ b/configs/mx6cuboxi_defconfig @@ -42,6 +42,7 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_DEFAULT_DEVICE_TREE="imx6dl-hummingboard2-emmc-som-v15" CONFIG_OF_LIST="imx6dl-hummingboard2-emmc-som-v15 imx6q-hummingboard2-emmc-som-v15" CONFIG_MULTI_DTB_FIT=y +CONFIG_SPL_OF_PLATDATA=y CONFIG_ENV_IS_IN_MMC=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y -- 2.20.1
[PATCH v2 4/6] mmc: fsl_esdhc_imx: add CD support when OF_PLATDATA is enabled
After enabling OF_PLATDATA support to both MMC and GPIO drivers add the support for card detection. Signed-off-by: Walter Lozano --- (no changes since v1) drivers/mmc/fsl_esdhc_imx.c | 27 ++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index f2509b5cc9..4448565b5a 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1498,7 +1498,32 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->bus_width = 4; else priv->bus_width = 1; - priv->non_removable = 1; + + if (dtplat->non_removable) + priv->non_removable = 1; + else + priv->non_removable = 0; + +#if CONFIG_IS_ENABLED(DM_GPIO) + if (!priv->non_removable) { + struct udevice *gpiodev; + struct driver_info *info; + + info = (struct driver_info *)dtplat->cd_gpios->node; + + ret = device_get_by_driver_info(info, ); + + if (ret) + return ret; + + ret = gpio_dev_request_index(gpiodev, gpiodev->name, "cd-gpios", +dtplat->cd_gpios->arg[0], GPIOD_IS_IN, +dtplat->cd_gpios->arg[1], >cd_gpio); + + if (ret) + return ret; + } +#endif #endif if (data) -- 2.20.1
[PATCH 5/6] drivers: rename more drivers to match compatible string
Continuing with the approach in commit rename additional drivers to allow the OF_PLATDATA support. Signed-off-by: Walter Lozano --- drivers/pinctrl/nxp/pinctrl-imx6.c | 6 -- drivers/video/imx/mxc_ipuv3_fb.c | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c index aafa3057ad..84004e5921 100644 --- a/drivers/pinctrl/nxp/pinctrl-imx6.c +++ b/drivers/pinctrl/nxp/pinctrl-imx6.c @@ -41,8 +41,8 @@ static const struct udevice_id imx6_pinctrl_match[] = { { /* sentinel */ } }; -U_BOOT_DRIVER(imx6_pinctrl) = { - .name = "imx6-pinctrl", +U_BOOT_DRIVER(fsl_imx6q_iomuxc) = { + .name = "fsl_imx6q_iomuxc", .id = UCLASS_PINCTRL, .of_match = of_match_ptr(imx6_pinctrl_match), .probe = imx6_pinctrl_probe, @@ -51,3 +51,5 @@ U_BOOT_DRIVER(imx6_pinctrl) = { .ops = _pinctrl_ops, .flags = DM_FLAG_PRE_RELOC, }; + +U_BOOT_DRIVER_ALIAS(fsl_imx6q_iomuxc, fsl_imx6dl_iomuxc) diff --git a/drivers/video/imx/mxc_ipuv3_fb.c b/drivers/video/imx/mxc_ipuv3_fb.c index 587d62f2d8..492bc3e829 100644 --- a/drivers/video/imx/mxc_ipuv3_fb.c +++ b/drivers/video/imx/mxc_ipuv3_fb.c @@ -660,8 +660,8 @@ static const struct udevice_id ipuv3_video_ids[] = { { } }; -U_BOOT_DRIVER(ipuv3_video) = { - .name = "ipuv3_video", +U_BOOT_DRIVER(fsl_imx6q_ipu) = { + .name = "fsl_imx6q_ipu", .id = UCLASS_VIDEO, .of_match = ipuv3_video_ids, .bind = ipuv3_video_bind, -- 2.20.1
[PATCH 6/6] mx6cuboxi: enable OF_PLATDATA
As both MMC and GPIO driver now supports OF_PLATDATA, enable it in defconfig in order to reduce the SPL footprint. Signed-off-by: Walter Lozano --- configs/mx6cuboxi_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig index cd4d4da43e..54e5b76f3c 100644 --- a/configs/mx6cuboxi_defconfig +++ b/configs/mx6cuboxi_defconfig @@ -42,6 +42,7 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_DEFAULT_DEVICE_TREE="imx6dl-hummingboard2-emmc-som-v15" CONFIG_OF_LIST="imx6dl-hummingboard2-emmc-som-v15 imx6q-hummingboard2-emmc-som-v15" CONFIG_MULTI_DTB_FIT=y +CONFIG_SPL_OF_PLATDATA=y CONFIG_ENV_IS_IN_MMC=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y -- 2.20.1
[PATCH 4/6] mmc: fsl_esdhc_imx: add CD support when OF_PLATDATA is enabled
After enabling OF_PLATDATA support to both MMC and GPIO drivers add the support for card detection. Signed-off-by: Walter Lozano --- drivers/mmc/fsl_esdhc_imx.c | 27 ++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index f2509b5cc9..4448565b5a 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1498,7 +1498,32 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->bus_width = 4; else priv->bus_width = 1; - priv->non_removable = 1; + + if (dtplat->non_removable) + priv->non_removable = 1; + else + priv->non_removable = 0; + +#if CONFIG_IS_ENABLED(DM_GPIO) + if (!priv->non_removable) { + struct udevice *gpiodev; + struct driver_info *info; + + info = (struct driver_info *)dtplat->cd_gpios->node; + + ret = device_get_by_driver_info(info, ); + + if (ret) + return ret; + + ret = gpio_dev_request_index(gpiodev, gpiodev->name, "cd-gpios", +dtplat->cd_gpios->arg[0], GPIOD_IS_IN, +dtplat->cd_gpios->arg[1], >cd_gpio); + + if (ret) + return ret; + } +#endif #endif if (data) -- 2.20.1
[PATCH 3/6] gpio: mxc_gpio: add OF_PLATDATA support
Continuing with the OF_PLATADATA support for iMX6 to reduce SPL footprint, add it to mxc_gpio. Thanks to this, it will be possible to enable card detection on MMC driver. Signed-off-by: Walter Lozano --- drivers/gpio/mxc_gpio.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 316dcc757b..fc49b5b577 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -13,6 +13,8 @@ #include #include #include +#include +#include enum mxc_gpio_direction { MXC_GPIO_DIRECTION_IN, @@ -22,6 +24,10 @@ enum mxc_gpio_direction { #define GPIO_PER_BANK 32 struct mxc_gpio_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + /* Put this first since driver model will copy the data here */ + struct dtd_gpio_mxc dtplat; +#endif int bank_index; struct gpio_regs *regs; }; @@ -306,8 +312,16 @@ static int mxc_gpio_bind(struct udevice *dev) * is statically initialized in U_BOOT_DEVICES.Here * will return. */ - if (plat) + + if (plat) { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dtd_gpio_mxc *dtplat = >dtplat; + + plat->regs = map_sysmem(dtplat->reg[0], dtplat->reg[1]); + plat->bank_index = dev->req_seq; +#endif return 0; + } addr = devfdt_get_addr(dev); if (addr == FDT_ADDR_T_NONE) @@ -350,6 +364,8 @@ U_BOOT_DRIVER(gpio_mxc) = { .bind = mxc_gpio_bind, }; +U_BOOT_DRIVER_ALIAS(gpio_mxc, fsl_imx6q_gpio) + #if !CONFIG_IS_ENABLED(OF_CONTROL) static const struct mxc_gpio_plat mxc_plat[] = { { 0, (struct gpio_regs *)GPIO1_BASE_ADDR }, -- 2.20.1
[PATCH 1/6] mmc: fsl_esdhc_imx: rename driver name to match ll_entry
As discussed in commit rename fsl_esdhc_imx driver to allow the OF_PLATDATA support. Signed-off-by: Walter Lozano --- drivers/mmc/fsl_esdhc_imx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 5b61eeb214..7d76b144a7 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1644,7 +1644,7 @@ static int fsl_esdhc_bind(struct udevice *dev) #endif U_BOOT_DRIVER(fsl_esdhc) = { - .name = "fsl-esdhc-mmc", + .name = "fsl_esdhc", .id = UCLASS_MMC, .of_match = fsl_esdhc_ids, .ops= _esdhc_ops, -- 2.20.1
[PATCH 2/6] mmc: fsl_esdhc_imx: add OF_PLATDATA support
In order to reduce the footprint of SPL by removing dtb and library overhead add OF_PLATDATA support to fsl_esdhc_imx. This initial approach does not support card detection, which will be enabled after adding OF_PLATDATA support to GPIO. Signed-off-by: Walter Lozano --- drivers/mmc/fsl_esdhc_imx.c | 67 + 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 7d76b144a7..f2509b5cc9 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -33,6 +33,9 @@ #include #include #include +#include +#include +#include #if !CONFIG_IS_ENABLED(BLK) #include "mmc_private.h" @@ -102,6 +105,11 @@ struct fsl_esdhc { }; struct fsl_esdhc_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + /* Put this first since driver model will copy the data here */ + struct dtd_fsl_esdhc dtplat; +#endif + struct mmc_config cfg; struct mmc mmc; }; @@ -1378,25 +1386,19 @@ __weak void init_clk_usdhc(u32 index) { } -static int fsl_esdhc_probe(struct udevice *dev) +static int fsl_esdhc_ofdata_to_platdata(struct udevice *dev) { - struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); - struct fsl_esdhc_plat *plat = dev_get_platdata(dev); +#if !CONFIG_IS_ENABLED(OF_PLATDATA) struct fsl_esdhc_priv *priv = dev_get_priv(dev); - const void *fdt = gd->fdt_blob; - int node = dev_of_offset(dev); - struct esdhc_soc_data *data = - (struct esdhc_soc_data *)dev_get_driver_data(dev); #if CONFIG_IS_ENABLED(DM_REGULATOR) struct udevice *vqmmc_dev; + int ret; #endif + const void *fdt = gd->fdt_blob; + int node = dev_of_offset(dev); + fdt_addr_t addr; unsigned int val; - struct mmc *mmc; -#if !CONFIG_IS_ENABLED(BLK) - struct blk_desc *bdesc; -#endif - int ret; addr = dev_read_addr(dev); if (addr == FDT_ADDR_T_NONE) @@ -1404,8 +1406,6 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->esdhc_regs = (struct fsl_esdhc *)addr; priv->dev = dev; priv->mode = -1; - if (data) - priv->flags = data->flags; val = dev_read_u32_default(dev, "bus-width", -1); if (val == 8) @@ -1469,6 +1469,40 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->vs18_enable = 1; } #endif +#endif + return 0; +} + +static int fsl_esdhc_probe(struct udevice *dev) +{ + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); + struct fsl_esdhc_plat *plat = dev_get_platdata(dev); + struct fsl_esdhc_priv *priv = dev_get_priv(dev); + struct esdhc_soc_data *data = + (struct esdhc_soc_data *)dev_get_driver_data(dev); + struct mmc *mmc; +#if !CONFIG_IS_ENABLED(BLK) + struct blk_desc *bdesc; +#endif + int ret; + +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dtd_fsl_esdhc *dtplat = >dtplat; + unsigned int val; + + priv->esdhc_regs = map_sysmem(dtplat->reg[0], dtplat->reg[1]); + val = plat->dtplat.bus_width; + if (val == 8) + priv->bus_width = 8; + else if (val == 4) + priv->bus_width = 4; + else + priv->bus_width = 1; + priv->non_removable = 1; +#endif + + if (data) + priv->flags = data->flags; /* * TODO: @@ -1520,9 +1554,11 @@ static int fsl_esdhc_probe(struct udevice *dev) return ret; } +#if !CONFIG_IS_ENABLED(OF_PLATDATA) ret = mmc_of_parse(dev, >cfg); if (ret) return ret; +#endif mmc = >mmc; mmc->cfg = >cfg; @@ -1647,6 +1683,7 @@ U_BOOT_DRIVER(fsl_esdhc) = { .name = "fsl_esdhc", .id = UCLASS_MMC, .of_match = fsl_esdhc_ids, + .ofdata_to_platdata = fsl_esdhc_ofdata_to_platdata, .ops= _esdhc_ops, #if CONFIG_IS_ENABLED(BLK) .bind = fsl_esdhc_bind, @@ -1655,4 +1692,6 @@ U_BOOT_DRIVER(fsl_esdhc) = { .platdata_auto_alloc_size = sizeof(struct fsl_esdhc_plat), .priv_auto_alloc_size = sizeof(struct fsl_esdhc_priv), }; + +U_BOOT_DRIVER_ALIAS(fsl_esdhc, fsl_imx6q_usdhc) #endif -- 2.20.1
[PATCH 0/6] mx6cuboxi: enable OF_PLATDATA with MMC support
The SPL in iMX6 boards is restricted to 68 KB as this is the free available space in OCRAM for most revisions. In this context, adding OF_CONTROL and DM increases the SPL size which could make it difficult to add specific features required for custom scenarios. These patches aim to take advantage of OF_PLATADATA in order to reduce the SPL size in this scenario, by parsing DT data to generate platdata structures, and thus removing the overhead caused by DT and related libraries. This series is focused in MMC driver, which is used for boot in boards such as Cubox-i. Also, in order to support CD, the OF_PLATDATA support is also implemented on GPIO driver. Walter Lozano (6): mmc: fsl_esdhc_imx: rename driver name to match ll_entry mmc: fsl_esdhc_imx: add OF_PLATDATA support gpio: mxc_gpio: add OF_PLATDATA support mmc: fsl_esdhc_imx: add CD support when OF_PLATDATA is enabled drivers: rename more drivers to match compatible string mx6cuboxi: enable OF_PLATDATA configs/mx6cuboxi_defconfig| 1 + drivers/gpio/mxc_gpio.c| 18 +- drivers/mmc/fsl_esdhc_imx.c| 94 +- drivers/pinctrl/nxp/pinctrl-imx6.c | 6 +- drivers/video/imx/mxc_ipuv3_fb.c | 4 +- 5 files changed, 103 insertions(+), 20 deletions(-) -- 2.20.1
[PATCH] dtoc: add coverage test for unicode error
Add an additional test to dtoc in order improve the coverage, specifically to take into account the case of unicode error when scanning drivers. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 14 +++--- tools/dtoc/test_dtoc.py| 18 ++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index c28768f4a2..c37db5b86e 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -145,8 +145,10 @@ class DtbPlatdata(object): U_BOOT_DRIVER_ALIAS(driver_alias, driver_name) value: Driver name declared with U_BOOT_DRIVER(driver_name) _links: List of links to be included in dm_populate_phandle_data() +_drivers_additional: List of additional drivers to use during scanning """ -def __init__(self, dtb_fname, include_disabled, warning_disabled): +def __init__(self, dtb_fname, include_disabled, warning_disabled, + drivers_additional=[]): self._fdt = None self._dtb_fname = dtb_fname self._valid_nodes = None @@ -157,6 +159,7 @@ class DtbPlatdata(object): self._drivers = [] self._driver_aliases = {} self._links = [] +self._drivers_additional = drivers_additional def get_normalized_compat_name(self, node): """Get a node's normalized compat name @@ -338,6 +341,10 @@ class DtbPlatdata(object): continue self.scan_driver(dirpath + '/' + fn) +for fn in self._drivers_additional: +print('Checking %s/%s' % (basedir, fn)) +self.scan_driver(basedir + '/' + fn) + def scan_dtb(self): """Scan the device tree to obtain a tree of nodes and properties @@ -654,7 +661,8 @@ class DtbPlatdata(object): self.out(''.join(self.get_buf())) -def run_steps(args, dtb_file, include_disabled, output, warning_disabled=False): +def run_steps(args, dtb_file, include_disabled, output, warning_disabled=False, + drivers_additional=[]): """Run all the steps of the dtoc tool Args: @@ -666,7 +674,7 @@ def run_steps(args, dtb_file, include_disabled, output, warning_disabled=False): if not args: raise ValueError('Please specify a command: struct, platdata') -plat = DtbPlatdata(dtb_file, include_disabled, warning_disabled) +plat = DtbPlatdata(dtb_file, include_disabled, warning_disabled, drivers_additional) plat.scan_drivers() plat.scan_dtb() plat.scan_tree() diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 169ecd6e6e..d750e52a34 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -13,6 +13,7 @@ import collections import os import struct import sys +import tempfile import unittest from dtoc import dtb_platdata @@ -829,3 +830,20 @@ U_BOOT_DEVICE(spl_test2) = { self.run_test(['invalid-cmd'], dtb_file, output) self.assertIn("Unknown command 'invalid-cmd': (use: struct, platdata)", str(e.exception)) + +def testUnicodeError(self): +"""Test running dtoc with an invalid unicode file + +To be able to perform this tests without adding a weird text file which +would produce issues when using checkpatch.pl or patman, generate the +file at runtime and then process it. +""" +dtb_file = get_dtb_file('dtoc_test_simple.dts') +output = tools.GetOutputFilename('output') +driver_fn = '/tmp/' + next(tempfile._get_candidate_names()) +with open(driver_fn, 'wb+') as df: +df.write(b'\x81') + +with test_util.capture_sys_output() as (stdout, stderr): +dtb_platdata.run_steps(['struct'], dtb_file, False, output, True, + ['tools/dtoc/dtoc_test_unicode_error.cxx']) -- 2.20.1
Re: dtoc code-coverage tests
Hi Simon, On 19/7/20 15:30, Simon Glass wrote: Hi Walter, I am seeing a test failure with code coverage - it is at 99%. It looks to be due to an exception that doesn't occur in the tests (UnicodeDecodeError). Could you please take a look? You should be able to test it by calling scan_driver with a known-bad unicode sequence. Yes, I noticed it during my recent work. Sure, I'm glad to help, I'll fix it and send a patch. Regards, Walter
[PATCH 2/2] dtoc: remove compatible string aliases support
After latest improvements in dtoc, compatible strings are checked against driver and driver alias list to get a valid driver name. With this new feature the list of compatible string aliases seems not useful any more. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 13 tools/dtoc/test_dtoc.py| 43 -- 2 files changed, 56 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index b1b082e508..c28768f4a2 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -139,9 +139,6 @@ class DtbPlatdata(object): _outfile: The current output file (sys.stdout or a real file) _warning_disabled: true to disable warnings about driver names not found _lines: Stashed list of output lines for outputting in the future -_aliases: Dict that hold aliases for compatible strings -key: First compatible string declared in a node -value: List of additional compatible strings declared in a node _drivers: List of valid driver names found in drivers/ _driver_aliases: Dict that holds aliases for driver names key: Driver alias declared with @@ -157,7 +154,6 @@ class DtbPlatdata(object): self._outfile = None self._warning_disabled = warning_disabled self._lines = [] -self._aliases = {} self._drivers = [] self._driver_aliases = {} self._links = [] @@ -483,10 +479,6 @@ class DtbPlatdata(object): prop.Widen(struct[name]) upto += 1 -struct_name, aliases = self.get_normalized_compat_name(node) -for alias in aliases: -self._aliases[alias] = struct_name - return structs def scan_phandles(self): @@ -549,11 +541,6 @@ class DtbPlatdata(object): self.out(';\n') self.out('};\n') -for alias, struct_name in self._aliases.items(): -if alias not in sorted(structs): -self.out('#define %s%s %s%s\n'% (STRUCT_PREFIX, alias, - STRUCT_PREFIX, struct_name)) - def output_node(self, node): """Output the C code for a node diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index edb3912e94..169ecd6e6e 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -290,7 +290,6 @@ struct dtd_sandbox_gpio { \tbool\t\tgpio_controller; \tfdt32_t\t\tsandbox_gpio_count; }; -#define dtd_sandbox_gpio_alias dtd_sandbox_gpio ''', data) self.run_test(['platdata'], dtb_file, output) @@ -555,48 +554,6 @@ void dm_populate_phandle_data(void) { self.assertIn("Node 'phandle-target' has no cells property", str(e.exception)) -def test_aliases(self): -"""Test output from a node with multiple compatible strings""" -dtb_file = get_dtb_file('dtoc_test_aliases.dts') -output = tools.GetOutputFilename('output') -self.run_test(['struct'], dtb_file, output) -with open(output) as infile: -data = infile.read() -self._CheckStrings(HEADER + ''' -struct dtd_compat1 { -\tfdt32_t\t\tintval; -}; -struct dtd_simple_bus { -\tfdt32_t\t\tintval; -}; -#define dtd_compat2_1_fred dtd_compat1 -#define dtd_compat3 dtd_compat1 -''', data) - -self.run_test(['platdata'], dtb_file, output) -with open(output) as infile: -data = infile.read() -self._CheckStrings(C_HEADER + ''' -static struct dtd_compat1 dtv_spl_test = { -\t.intval\t\t\t= 0x1, -}; -U_BOOT_DEVICE(spl_test) = { -\t.name\t\t= "compat1", -\t.platdata\t= _spl_test, -\t.platdata_size\t= sizeof(dtv_spl_test), -}; - -static struct dtd_simple_bus dtv_spl_test2 = { -\t.intval\t\t\t= 0x1, -}; -U_BOOT_DEVICE(spl_test2) = { -\t.name\t\t= "simple_bus", -\t.platdata\t= _spl_test2, -\t.platdata_size\t= sizeof(dtv_spl_test2), -}; - -''' + C_EMPTY_POPULATE_PHANDLE_DATA, data) - def test_addresses64(self): """Test output from a node with a 'reg' property with na=2, ns=2""" dtb_file = get_dtb_file('dtoc_test_addr64.dts') -- 2.20.1
[PATCH 0/2] dtoc: improve compatible string aliases support
This series adds additional improvements to dtoc, which are focused in compatible string aliases. First, enhance dtoc to use not only the first but all the compatible strings in a dtb node in order to find a valid driver name. Second, remove compatible string aliases from dtoc which were used to generate mappings between struct names. This is not longer needed since now a valid driver name is used. Walter Lozano (2): dtoc: look for compatible string aliases in driver list dtoc: remove compatible string aliases support tools/dtoc/dtb_platdata.py | 58 +--- tools/dtoc/dtoc_test_aliases.dts | 5 +++ tools/dtoc/test_dtoc.py | 39 +++-- 3 files changed, 32 insertions(+), 70 deletions(-) -- 2.20.1
[PATCH 1/2] dtoc: look for compatible string aliases in driver list
Currently dtoc checks if the first compatible string in a dtb node matches either a driver o driver alias name, without taking into account any other compatible string in the list. In the case that no driver matches the first compatible string a warning is printed and the U_BOOT_DEVICE is not being declared correctly. This patch adds dtoc's support for try all the compatible strings in the dtb node, in an effort to find the correct driver. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 45 tools/dtoc/dtoc_test_aliases.dts | 5 tools/dtoc/test_dtoc.py | 20 +++--- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index c148c49625..b1b082e508 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -111,21 +111,17 @@ def get_value(ftype, value): return '%#x' % value def get_compat_name(node): -"""Get a node's first compatible string as a C identifier +"""Get the node's list of compatible string as a C identifiers Args: node: Node object to check Return: -Tuple: -C identifier for the first compatible string -List of C identifiers for all the other compatible strings -(possibly empty) +List of C identifiers for all the compatible strings """ compat = node.props['compatible'].value -aliases = [] -if isinstance(compat, list): -compat, aliases = compat[0], compat[1:] -return conv_name_to_c(compat), [conv_name_to_c(a) for a in aliases] +if not isinstance(compat, list): +compat = [compat] +return [conv_name_to_c(c) for c in compat] class DtbPlatdata(object): @@ -169,7 +165,7 @@ class DtbPlatdata(object): def get_normalized_compat_name(self, node): """Get a node's normalized compat name -Returns a valid driver name by retrieving node's first compatible +Returns a valid driver name by retrieving node's list of compatible string as a C identifier and performing a check against _drivers and a lookup in driver_aliases printing a warning in case of failure. @@ -183,19 +179,24 @@ class DtbPlatdata(object): In case of no match found, the return will be the same as get_compat_name() """ -compat_c, aliases_c = get_compat_name(node) -if compat_c not in self._drivers: -compat_c_old = compat_c -compat_c = self._driver_aliases.get(compat_c) -if not compat_c: -if not self._warning_disabled: -print('WARNING: the driver %s was not found in the driver list' - % (compat_c_old)) -compat_c = compat_c_old -else: -aliases_c = [compat_c_old] + aliases_c +compat_list_c = get_compat_name(node) + +for compat_c in compat_list_c: +if not compat_c in self._drivers: +compat_c = self._driver_aliases.get(compat_c) +if not compat_c: +continue + +aliases_c = compat_list_c +if compat_c in aliases_c: +aliases_c.remove(compat_c) +return compat_c, aliases_c + +if not self._warning_disabled: +print('WARNING: the driver %s was not found in the driver list' + % (compat_list_c[0])) -return compat_c, aliases_c +return compat_list_c[0], compat_list_c[1:] def setup_output(self, fname): """Set up the output destination diff --git a/tools/dtoc/dtoc_test_aliases.dts b/tools/dtoc/dtoc_test_aliases.dts index e545816f4e..ae33716863 100644 --- a/tools/dtoc/dtoc_test_aliases.dts +++ b/tools/dtoc/dtoc_test_aliases.dts @@ -14,4 +14,9 @@ intval = <1>; }; + spl-test2 { + u-boot,dm-pre-reloc; + compatible = "compat1", "simple_bus"; + intval = <1>; + }; }; diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 3c8e343b1f..edb3912e94 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -144,18 +144,18 @@ class TestDtoc(unittest.TestCase): prop = Prop(['rockchip,rk3399-sdhci-5.1', 'arasan,sdhci-5.1']) node = Node({'compatible': prop}) -self.assertEqual(('rockchip_rk3399_sdhci_5_1', ['arasan_sdhci_5_1']), +self.assertEqual((['rockchip_rk3399_sdhci_5_1', 'arasan_sdhci_5_1']), get_compat_name(node)) prop = Prop(['rockchip,rk3399-sdhci-5.1']) node = Node({'compatible': prop}) -self.assertEqual(('rockchip_rk3399_sdhci_5_1', []), +self.assertEqual((['rockchip_rk3399_sdhci_5_1']),
Re: [RFC PATCH v2 0/3] RFC: tiny-dm: Proposal for using driver model in SPL
Hi Simon, On 2/7/20 18:10, Simon Glass wrote: This series provides a proposed enhancement to driver model to reduce overhead in SPL. These patches should not be reviewed other than to comment on the approach. The code is all lumped together in a few patches and so cannot be applied as is. For now, the source tree is available at: https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/dtoc-working Comments welcome! Benefits (good news) As an example of the impact of tiny-dm, chromebook_jerry is converted to use it. This shows approximately a 30% reduction in code and data size and a 85% reduction in malloc() space over of-platdata: text data bss dec hex filename 25248 1836 12 2709669d8 spl/u-boot-spl (original with DT) 19727 3436 12 231755a87 spl/u-boot-spl (OF_PLATDATA) 78%187%100% 86% as %age of original 13784 1408 12 152043b64 spl/u-boot-spl (SPL_TINY) 70% 41%100% 66% as %age of platdata 55% 77%100% 56% as %age of original SPL malloc() usage drops from 944 bytes (OF_PLATDATA) to 116 (SPL_TINY). Overall the 'overhead' of tiny-dm is much less than the full driver model. Code size is currently about 600 bytes for these functions on Thumb2: 0054 T tiny_dev_probe 0034 T tiny_dev_get_by_drvdata 0024 T tiny_dev_find 001a T tiny_dev_get 003c T tinydev_alloc_data 002a t tinydev_lookup_data 0022 T tinydev_ensure_data 0014 T tinydev_get_data 0004 T tinydev_get_parent Effort (bad news) - Unfortunately it is quite a bit of work to convert drivers over to tiny-dm. First, the of-platdata conversion must be done. But on top of that, tiny-dm needs entirely separate code for dealing with devices. This means that instead of 'struct udevice' and 'struct uclass' there is just 'struct tinydev'. Each driver and uclass must be modified to support both, pulling common code into internal static functions. Another option -- Note: It is assumed that any board that is space-contrained should use of-platdata in SPL (see doc/driver-model/of-plat.rst). This is shown to reduce device-tree overhead by approximately 4KB. Designing tiny-dm has suggested a number of things that could be changed in the current driver model to make it more space-efficient for TPL and SPL. The ones with least impact on driver code are (CS=reduces code size, DS=reduces data size): CS - drop driver_bind() and create devices (struct udevice) at build-time CS - allocate all device- and uclass-private data at build-time CS - remove all but the basic operations for each uclass (e.g. SPI flash only supports reading) DS - use 8-bit indexes instead of 32/64-bit pointers for device pointers possible since these are created at build-time) DS - use singly-linked lists DS - use 16-bit offsets to private data, instead of 32/64-bit pointers (possible since it is all in SRAM relative to malloc() base, presumably word-aligned and < 256KB) DS - move private pointers into a separate data structure so that NULLs are not stored CS / DS - Combine req_seq and seq and calculate the new value at build-time More difficult are: DS - drop some of the lesser-used driver and uclass methods DS - drop all uclass methods except init() DS - drop all driver methods except probe() CS / DS - drop uclasses and require drivers to manually call uclass functions Even with all of this we would not reach tiny-dm and it would muddy up the driver-model datas structures. But we might come close to tiny-dm on size and there are some advantages: - much of the benefit would apply to all boards that use of-platdata (i.e. with very little effort on behalf of board maintainers) - the impact on the code base is much less (we keep a single, unified driver mode in SPL and U-Boot proper) Overall I think it is worth looking at this option. While it doesn't have the 'nuclear' impact of tiny-dm, neither does it mess with the U-Boot driver code as much and it is easier to learn. Thanks for your hard work on this topic. I think that there is great value in this research and in this conclusion. It is clear that there two different approaches, but I feel that the improvement to the current DM implementation would have a higher impact in the community. Since the first version of this proposal I have been thinking in a solution that takes some of the advantages of tiny-dm idea but that does not require so much effort. This seems to be aligned with what you have been explaining in this section. I found interesting your proposal about simplification some data structures. In this sense one of my ideas, a bit biased by some of the improvements in dtoc, is to change
Re: [RFC 1/4] dtoc: add POC for dtb shrink
Hi Rasmus, On 7/7/20 11:53, Rasmus Villemoes wrote: On 07/07/2020 16.32, Walter Lozano wrote: Hi Rasmus, On 7/7/20 11:15, Rasmus Villemoes wrote: On 19/06/2020 23.11, Walter Lozano wrote: Some additional reduction could be possible by only keeping the nodes for whose compatible string is supported by any enabled driver. However, this requires to add extra logic to parse config files and map configuration to compatible strings. If this can be done after building the U-Boot (or SPL) ELF, can't it just be done by doing 'grep -a' on that? Or, probably a little more efficient, running "strings | grep -E '^[a-zA-Z0-9,._-]*' | sort -u", slurping in the output of that in a python set() and just looking there. Thanks for your review and suggestion. Your approach is interesting, however, I wonder, won't we get a lot of strings which are not compatible strings? How could be filter this list to only include those strings that are compatible strings? Does it matter? You have a dt node containing 'compatible = "acme,frobnozzle"', so you want to know if any enabled driver has "acme,frobnozzle". Sure, the brute-force grep'ing will produce lots and lots of irrelevant strings, but the chance of a false positive (acme,frobnozzle appearing, but not from some driver's compatible strings) should be quite low, and false negatives can't (and must not, of course) happen AFAICS. I agree with you regarding the fact that the chances of a false positive are low, and also causes no big issue, just some node we don't remove. However as much of the support in dtoc is already there for other features I I think is cleaner in that way. Not sure what other people think. Also the idea if parsing config and Makefiles would be useful to only process file drivers which are going to be used, and prepare for instance the compatible string list as described in "[RFC 3/4] dtoc: add support for generate stuct udevice_id", which can be found in https://patchwork.ozlabs.org/project/uboot/patch/20200619211140.5081-4-walter.loz...@collabora.com/ Do you think we can handle this in some other more efficient way? I haven't read these patches very closely, just stumbled on the above. I do think that the job of parsing Kconfig files is best left to Kconfig, the job of parsing Makefiles is best left to make, and the job of processing all the #ifdefery/CONFIG_IS_ENABLED stuff is best left to the compiler (preprocessor). Trying to predict which code will or will not be included in a given image in any way other than by building that image sounds quite error-prone. But I may very well not have understood what you're proposing. Yes, I also agree with you, that this could be error-prone, that is my main concern, and that is the reason for sending this RFC, to explore this (and possible other) ideas or approaches to reduce the footprint. To elaborate a bit more, I think the scanning the Makefiles could be error-prone, and could lead to some file driver not being taken into account, which could lead to runtime issues. On the other hand, I don't see much of a problem to use some macros which will be implemented in code generated by dtoc, as this idea is already used for OF_PLATDATA. If you see some specific issue please let me know. Thanks for sharing you thoughts, it is very useful. I will keep thinking about your suggestions and different alternatives. Regards, Walter Rasmus
Re: [RFC 1/4] dtoc: add POC for dtb shrink
Hi Rasmus, On 7/7/20 11:15, Rasmus Villemoes wrote: On 19/06/2020 23.11, Walter Lozano wrote: Some additional reduction could be possible by only keeping the nodes for whose compatible string is supported by any enabled driver. However, this requires to add extra logic to parse config files and map configuration to compatible strings. If this can be done after building the U-Boot (or SPL) ELF, can't it just be done by doing 'grep -a' on that? Or, probably a little more efficient, running "strings | grep -E '^[a-zA-Z0-9,._-]*' | sort -u", slurping in the output of that in a python set() and just looking there. Thanks for your review and suggestion. Your approach is interesting, however, I wonder, won't we get a lot of strings which are not compatible strings? How could be filter this list to only include those strings that are compatible strings? Also the idea if parsing config and Makefiles would be useful to only process file drivers which are going to be used, and prepare for instance the compatible string list as described in "[RFC 3/4] dtoc: add support for generate stuct udevice_id", which can be found in https://patchwork.ozlabs.org/project/uboot/patch/20200619211140.5081-4-walter.loz...@collabora.com/ Do you think we can handle this in some other more efficient way? Regards, Walter
Re: [RFC 3/4] dtoc: add support for generate stuct udevice_id
Hi Simon On 6/7/20 16:21, Simon Glass wrote: Hi Walter, On Fri, 19 Jun 2020 at 15:12, Walter Lozano wrote: Based on several reports there is an increasing concern in the impact of adding additional features to drivers based on compatible strings. A good example of this situation is found in [1]. In order to reduce this impact and as an initial step for further reduction, propose a new way to declare compatible strings, which allows to only include the useful ones. What are the useful ones? The useful ones would be those that are used by the selected DTB by the current configuration. The idea of this patch is to declare all the possible compatible strings in a way that dtoc can generate code for only those which are going to be used, and in this way avoid lots of #ifdef like the ones shows in http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ The idea is to define compatible strings in a way to be easily parsed by dtoc, which will be responsible to build struct udevice_id [] based on the compatible strings present in the dtb. Additional features can be easily added, such as define constants depending on the presence of compatible strings, which allows to enable code blocks only in such cases without the need of adding additional configuration options. [1] http://patchwork.ozlabs.org/project/uboot/patch/20200525202429.2146-1-ag...@denx.de/ Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 32 1 file changed, 32 insertions(+) I think dtoc should be able to parse the compatible strings as they are today - e.g. see the tiny-dm stuff. Yes, I agree. My idea is that dtoc parses compatible strings as they are today but also in this new way. The reason for this is to allow dtoc to generate the code to include the useful compatible strings. Of course, this only makes sense if the idea of generating the compatible string associated code is accepted. What do you think? Regards, Walter Regards, Simon
Re: [RFC 1/4] dtoc: add POC for dtb shrink
Hi Simon, Thanks for your time. On 6/7/20 16:21, Simon Glass wrote: Hi Walter, On Fri, 19 Jun 2020 at 15:11, Walter Lozano wrote: Based on several reports and discussions [1], [2] it is clear that U-Boot's footprint is always a concern, and any kind of reduction is an improvement. In particular dtb is one of the sources of footprint increment, as U-Boot uses the same dtb as Linux. However is interesting to note that U-Boot does not require all the nodes and properties declared in it. Some improvements in this sense are already present, such as removing properties based on configuration and using specific "u-boot" properties to keep only specific node in SPL. However, this require manual configuration. Additionally reducing dtb, will allow ATF for better handing FDT buffer, which is an issue in some contexts [3]. In order to improve this situation, this patch adds a proof of concept for dtb shrink. The idea behind this is simple, remove all the nodes from dtb which compatible string is not supported by any driver present. This approach makes sense for those configuration where Linux is expected to have its own dtb. This patch is based on the work of Simon Glass present in [4] which adds support to dtoc for parsing compatible strings. Some early untested results shows that the reduction in size is 50 % in case of mx6_cuboxi_defconfig, which is promising. Some additional reduction could be possible by only keeping the nodes for whose compatible string is supported by any enabled driver. However, this requires to add extra logic to parse config files and map configuration to compatible strings. This proof of concept uses fdtgrep to implement the node removal, but the idea is to implement this logic inside the dtoc for better handling. [1] http://patchwork.ozlabs.org/project/uboot/patch/20200525093539.1.Ibf2d19439cde35e39192a9d4a8dad23539fae2e6@changeid/ [2] http://u-boot.10912.n7.nabble.com/PATCH-v1-00-15-add-basic-driver-support-for-broadcom-NS3-soc-tt412411.html#none [3] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/4512 [4] https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/dtoc-working Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 42 +++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 21cce5afb5..1df13b2cd2 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -399,7 +399,10 @@ class DtbPlatdata(object): """Scan the driver folders to build a list of driver names and possible aliases """ -for (dirpath, dirnames, filenames) in os.walk('/home/sglass/u'): +basedir = sys.argv[0].replace('tools/dtoc/dtoc', '') Instead of this logic, can we pass the source-tree location into dtoc with a flag? It could default to the current dir perhaps. Sure, no problem at all. +if basedir == '': +basedir = './' +for (dirpath, dirnames, filenames) in os.walk(basedir): for fn in filenames: if not fn.endswith('.c'): continue @@ -802,6 +805,32 @@ class DtbPlatdata(object): self.out(''.join(self.get_buf())) self.close_output() +def shrink(self): +"""Generate a shrunk version of DTB bases on valid drivers + +This function removes nodes from dtb which compatible string is not +found in drivers. The output is saved in a file with suffix name -shrink.dtb +""" +compat = [] +cmd = './tools/fdtgrep ' +#print(self._drivers) +for dr in self._drivers.values(): +compat = compat + dr.compat + +for cp in compat: +#print(cp) +cmd += ' -c ' + cp + +cmd += ' -O dtb -o ' + self._dtb_fname.replace('.dtb', '') + '-shrink.dtb ' + self._dtb_fname + +if False: +with open('dt_shrink.sh', 'w+') as script: +script.write(cmd) + +os.system(cmd) + +return + def run_steps(args, dtb_file, config_file, include_disabled, output): """Run all the steps of the dtoc tool @@ -816,6 +845,10 @@ def run_steps(args, dtb_file, config_file, include_disabled, output): if not args: raise ValueError('Please specify a command: struct, platdata') +skip_scan = False +if args == ['shrink']: +skip_scan = True I think that would be better as a positive variable, like 'scan'. Yes, I agree. The idea was to test the general idea, and check if it could be useful. + plat = DtbPlatdata(dtb_file, config_file, include_disabled) plat.scan_drivers() plat.scan_dtb() @@ -823,14 +856,17 @@ def run_steps(args, dtb_file, config_file, include_disabled, output): plat.scan_config() plat.scan_reg_sizes() Are those
Re: [RFC 2/4] dtoc: add initial support for deleting DTB nodes
Hi Simon, On 6/7/20 16:21, Simon Glass wrote: Hi Walter, On Fri, 19 Jun 2020 at 15:12, Walter Lozano wrote: This patch introduce a test for deleting DTB nodes using Python library. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 28 tools/dtoc/fdt.py | 3 +++ 2 files changed, 31 insertions(+) This test should go in test_dtoc.py Thanks for checking this series. Yes, you are right, this was an early version in order to check with you and Tom if the idea makes any sense. Regards, Walter
Re: [PATCH v1 1/1] mkimage: Fix error message if write less data then expected
Hi Mylene, On 7/7/20 05:25, Mylene Josserand wrote: Hi, On 6/9/20 8:49 PM, Walter Lozano wrote: Hi Mylene, Thanks for you patch. On 5/6/20 05:01, Mylène Josserand wrote: Add a new error message in case the size of data written are shorter than the one expected. Currently, it will lead to the following error message: "mkimage: Write error on uImage: Success" This is not explicit when the error is because the device doesn't have enough space. Let's use a "No space left on the device" error message in that case. Signed-off-by: Mylène Josserand --- tools/mkimage.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tools/mkimage.c b/tools/mkimage.c index d2cd1917874..ef0cc889a92 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -674,7 +674,7 @@ copy_file (int ifd, const char *datafile, int pad) int zero = 0; uint8_t zeros[4096]; int offset = 0; - int size; + int size, ret; struct image_type_params *tparams = imagetool_get_type(params.type); memset(zeros, 0, sizeof(zeros)); @@ -730,9 +730,15 @@ copy_file (int ifd, const char *datafile, int pad) } size = sbuf.st_size - offset; - if (write(ifd, ptr + offset, size) != size) { - fprintf (stderr, "%s: Write error on %s: %s\n", - params.cmdname, params.imagefile, strerror(errno)); + + ret = write(ifd, ptr + offset, size); + if (ret != size) { + if (ret < 0) + fprintf (stderr, "%s: Write error on %s: %s\n", + params.cmdname, params.imagefile, strerror(errno)); + else if (ret < size) + fprintf (stderr, "%s: No space left on the device\n", + params.cmdname); Thanks for improving the error message, it is much more clear now. The only question I have is if "no space left on the device" is the only possible cause for this situation, for sure it is the most probable. Thanks for your review. Indeed, it is the most probable I guess. Yes, I agree. However, in order to be more accurate, and probably avoid future patches to correct a misleading message I would improve the error message with something like "Fewer bytes written than expected, probably no space left on the device" or something similar. What do you think? Besides that Reviewed-by: Walter Lozano exit (EXIT_FAILURE); } Regards, Walter
Re: [PATCH v5 04/14] dtoc: add support to scan drivers
Hi Simon, On 3/7/20 13:08, Simon Glass wrote: Hi Walter, On Fri, 3 Jul 2020 at 05:07, Walter Lozano wrote: Currently dtoc scans dtbs to convert them to struct platdata and to generate U_BOOT_DEVICE entries. These entries need to be filled with the driver name, but at this moment the information used is the compatible name present in the dtb. This causes that only nodes with a compatible name that matches a driver name generate a working entry. In order to improve this behaviour, this patch adds to dtoc the capability of scan drivers source code to generate a list of valid driver names and aliases. This allows to generate U_BOOT_DEVICE entries using valid driver names and rise a warning in the case a name is not valid. Signed-off-by: Walter Lozano --- (no changes since v1) tools/dtoc/dtb_platdata.py| 91 +-- tools/dtoc/dtoc_test_driver_alias.dts | 20 ++ tools/dtoc/test_dtoc.py | 33 ++ 3 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 tools/dtoc/dtoc_test_driver_alias.dts This seems to do the trick, thanks! But there is one problem remaining - lots of warning output from the various tests: https://pastebin.com/K7GBPiqC The warnings are removed by the next patch in the series, "[PATCH v4 05/14] dtoc: add option to disable warnings", by adding additional arguments. I thought having these two things in different patches made sense, but If you prefer I can re send the series with those two patches squashed. Please confirm. Regards, Walter
Re: [PATCH v4 04/14] dtoc: add support to scan drivers
On 2/7/20 16:47, Simon Glass wrote: Hi Walter, On Thu, 25 Jun 2020 at 19:43, Simon Glass wrote: On Wed, 24 Jun 2020 at 22:10, Walter Lozano wrote: Currently dtoc scans dtbs to convert them to struct platdata and to generate U_BOOT_DEVICE entries. These entries need to be filled with the driver name, but at this moment the information used is the compatible name present in the dtb. This causes that only nodes with a compatible name that matches a driver name generate a working entry. In order to improve this behaviour, this patch adds to dtoc the capability of scan drivers source code to generate a list of valid driver names and aliases. This allows to generate U_BOOT_DEVICE entries using valid driver names and rise a warning in the case a name is not valid. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 91 -- tools/dtoc/test_dtoc.py| 33 ++ 2 files changed, 120 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass Unfortunately this patch seems to be missing a file dtoc_test_driver_alias.dts without which the tests fail. Can you please resend just this patch? Sorry for the mistake. I've just sent the patch. Thanks for your time and deep review. Regards, Walter
[PATCH v5 04/14] dtoc: add support to scan drivers
Currently dtoc scans dtbs to convert them to struct platdata and to generate U_BOOT_DEVICE entries. These entries need to be filled with the driver name, but at this moment the information used is the compatible name present in the dtb. This causes that only nodes with a compatible name that matches a driver name generate a working entry. In order to improve this behaviour, this patch adds to dtoc the capability of scan drivers source code to generate a list of valid driver names and aliases. This allows to generate U_BOOT_DEVICE entries using valid driver names and rise a warning in the case a name is not valid. Signed-off-by: Walter Lozano --- (no changes since v1) tools/dtoc/dtb_platdata.py| 91 +-- tools/dtoc/dtoc_test_driver_alias.dts | 20 ++ tools/dtoc/test_dtoc.py | 33 ++ 3 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 tools/dtoc/dtoc_test_driver_alias.dts diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index bc0de426a9..ae8674d85c 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -13,6 +13,8 @@ static data. import collections import copy +import os +import re import sys from dtoc import fdt @@ -143,6 +145,11 @@ class DtbPlatdata(object): _aliases: Dict that hold aliases for compatible strings key: First compatible string declared in a node value: List of additional compatible strings declared in a node +_drivers: List of valid driver names found in drivers/ +_driver_aliases: Dict that holds aliases for driver names +key: Driver alias declared with +U_BOOT_DRIVER_ALIAS(driver_alias, driver_name) +value: Driver name declared with U_BOOT_DRIVER(driver_name) """ def __init__(self, dtb_fname, include_disabled): self._fdt = None @@ -152,6 +159,38 @@ class DtbPlatdata(object): self._outfile = None self._lines = [] self._aliases = {} +self._drivers = [] +self._driver_aliases = {} + +def get_normalized_compat_name(self, node): +"""Get a node's normalized compat name + +Returns a valid driver name by retrieving node's first compatible +string as a C identifier and performing a check against _drivers +and a lookup in driver_aliases printing a warning in case of failure. + +Args: +node: Node object to check +Return: +Tuple: +Driver name associated with the first compatible string +List of C identifiers for all the other compatible strings +(possibly empty) +In case of no match found, the return will be the same as +get_compat_name() +""" +compat_c, aliases_c = get_compat_name(node) +if compat_c not in self._drivers: +compat_c_old = compat_c +compat_c = self._driver_aliases.get(compat_c) +if not compat_c: +print('WARNING: the driver %s was not found in the driver list' + % (compat_c_old)) +compat_c = compat_c_old +else: +aliases_c = [compat_c_old] + aliases_c + +return compat_c, aliases_c def setup_output(self, fname): """Set up the output destination @@ -246,6 +285,49 @@ class DtbPlatdata(object): return PhandleInfo(max_args, args) return None +def scan_driver(self, fn): +"""Scan a driver file to build a list of driver names and aliases + +This procedure will populate self._drivers and self._driver_aliases + +Args +fn: Driver filename to scan +""" +with open(fn) as fd: +buff = fd.read() + +# The following re will search for driver names declared as +# U_BOOT_DRIVER(driver_name) +drivers = re.findall('U_BOOT_DRIVER\((.*)\)', buff) + +for driver in drivers: +self._drivers.append(driver) + +# The following re will search for driver aliases declared as +# U_BOOT_DRIVER_ALIAS(alias, driver_name) +driver_aliases = re.findall('U_BOOT_DRIVER_ALIAS\(\s*(\w+)\s*,\s*(\w+)\s*\)', +buff) + +for alias in driver_aliases: # pragma: no cover +if len(alias) != 2: +continue +self._driver_aliases[alias[1]] = alias[0] + +def scan_drivers(self): +"""Scan the driver folders to build a list of driver names and aliases + +This procedure will populate self._drivers and self._driver_aliases + +""" +basedir = sys.argv[0].replace('tools/dtoc/dtoc', '') +if basedir
Re: [PATCH v6 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor
On 30/6/20 13:19, Walter Lozano wrote: Hi Heiko On 22/5/20 06:08, Heiko Schocher wrote: save the GPIOD_ flags also in the gpio descriptor. Signed-off-by: Heiko Schocher Reviewed-by: Patrick Delaunay Fixes: 788ea834124b ("gpio: add function _dm_gpio_set_dir_flags") Thanks for this fix, without it the MMC driver of my iMX6 Hummingboard produces "Card did not respond to voltage select!" and does not work. Tested-by: Walter Lozano Regards, Walter --- Changes in v6: - add reviewed by from Patrick and Fixes tag Changes in v5: - add comment from patrick, update the descriptor flags in _dm_gpio_set_dir_flags() if setting direction was OK. Changes in v4: - new in version 4 Changes in v3: None Changes in v2: None drivers/gpio/gpio-uclass.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 9eeab22eef..f016532354 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -600,6 +600,10 @@ static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) } } + /* save the flags also in descriptor */ + if (!ret) + desc->flags = flags; + return ret; } @@ -615,10 +619,6 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) flags |= desc->flags; ret = _dm_gpio_set_dir_flags(desc, flags); - /* update the descriptor flags */ - if (ret) - desc->flags = flags; - return ret; } Regards, Walter
Re: [PATCH] rockchip: rk3399: allow deselecting SPL_ATF_NO_PLATFORM_PARAM
On 27/6/20 11:36, Kever Yang wrote: On 2020/6/16 上午7:30, Hugh Cole-Baker wrote: SPL_ATF_NO_PLATFORM_PARAM is selected by default for RK3399 configs, to guard against issues when used with TF-A versions that perform insufficient validation on the platform parameter. However, since commit 8109f738ffa7 "rockchip: increase FDT buffer size" in TF-A, passing a device tree as platform parameter no longer causes problems for upstream TF-A for RK3399. Since SPL_ATF_NO_PLATFORM_PARAM doesn't need to be selected when using upstream TF-A, change the Kconfig option from select to imply. It'll still default to being selected but can be deselected by a user if they know they will be using a compatible version of TF-A. Signed-off-by: Hugh Cole-Baker Reviewed-by: Kever Yang Thanks for this patch. Now with the latest TF-A and deselecting SPL_ATF_NO_PLATFORM_PARAM is it possible to get console output at the correct baudrate on TF. Tested-by: Walter Lozano Regards, Walter --- For some background, see this thread on the TF-A list [1]. Since the corresponding required change isn't in a tagged version of TF-A yet, and I don't know how many RK3399 boards are normally used with older TF-A versions which required this option, I think it's safest to keep the default as not sending a platform param to TF-A. Once the next TF-A version is released, SPL_ATF_NO_PLATFORM_PARAM could be turned off in defconfigs for boards that use the latest upstream TF-A. [1]: https://lists.trustedfirmware.org/pipermail/tf-a/2020-June/000502.html arch/arm/mach-rockchip/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 0cb1f23d0f3..e2b63265846 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -207,7 +207,6 @@ config ROCKCHIP_RK3399 select SUPPORT_TPL select SPL select SPL_ATF - select SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF select SPL_BOARD_INIT if SPL select SPL_LOAD_FIT select SPL_CLK if SPL @@ -232,6 +231,7 @@ config ROCKCHIP_RK3399 imply PRE_CONSOLE_BUFFER imply ROCKCHIP_COMMON_BOARD imply ROCKCHIP_SDRAM_COMMON + imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF imply SPL_ROCKCHIP_COMMON_BOARD imply TPL_SERIAL_SUPPORT imply TPL_LIBCOMMON_SUPPORT
Re: [PATCH v6 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor
Hi Heiko On 22/5/20 06:08, Heiko Schocher wrote: save the GPIOD_ flags also in the gpio descriptor. Signed-off-by: Heiko Schocher Reviewed-by: Patrick Delaunay Fixes: 788ea834124b ("gpio: add function _dm_gpio_set_dir_flags") Thanks for this fix, without it the MMC driver of my iMX6 Hummingboard produces "Card did not respond to voltage select!" and does not work. --- Changes in v6: - add reviewed by from Patrick and Fixes tag Changes in v5: - add comment from patrick, update the descriptor flags in _dm_gpio_set_dir_flags() if setting direction was OK. Changes in v4: - new in version 4 Changes in v3: None Changes in v2: None drivers/gpio/gpio-uclass.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 9eeab22eef..f016532354 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -600,6 +600,10 @@ static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) } } + /* save the flags also in descriptor */ + if (!ret) + desc->flags = flags; + return ret; } @@ -615,10 +619,6 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) flags |= desc->flags; ret = _dm_gpio_set_dir_flags(desc, flags); - /* update the descriptor flags */ - if (ret) - desc->flags = flags; - return ret; } Regards, Walter
Re: [RFC 0/4] drivers: footprint reduction proposal
Hi Tom, On 22/6/20 12:25, Walter Lozano wrote: Hi Tom, On 22/6/20 11:20, Tom Rini wrote: On Mon, Jun 22, 2020 at 11:12:40AM -0300, Walter Lozano wrote: Hi Tom, On 19/6/20 18:48, Tom Rini wrote: On Fri, Jun 19, 2020 at 06:11:36PM -0300, Walter Lozano wrote: Based on several reports and discussions it is clear that U-Boot's footprint is always a concern, and any kind of reduction is an improvement. This series is a proposal to help reducing the footprint by parsing information provided in DT and drivers in different ways and adding additional intelligence to dtoc. The current version implements the basic functionality in dtoc but this is no fully integrated, however it will allow us to discuss this approach. Firstly, based on the compatible strings found in drivers, include only DT nodes which are supported by any driver present in U-Boot. Secondly, generate struct udevice_id entries only for nodes present in DT, which will allow to avoid including additional data. These are the first steps for further improvements as proposed in the specific patches in this series. This work is based on the work of Simon Glass present in [1] which adds support to dtoc for parsing compatible strings. [1] https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/dtoc-working I applied this series on top of the above tree, but there's no rule for so is something missing? Thanks! Thanks for taking the time to check this RFC. As you pointed, the Makefile needs to be tweaked in order to be able to run a build, that is what I meant by "not fully integrated", also some additional stuff are missing. However, I thought that sending this RFC explaining the idea will be nice in order to confirm if the approaches proposed make sense for the community and at least the one to handle compatible strings is different from the linker list suggestion. I think I like the idea, but I need to give a build a spin and poke things harder. What do I need to do to manually have this build+link? Thanks! Well, I don't think this version will give you something to fully test, as there are several pieces missing, but the fact you think you like the idea is good starting point. Just to do some testing you can try Shrink a dtb with ./tools/dtoc/dtoc shrink -d u-boot.dtb this will shrink u-boot.dtb and create u-boot-shrink.dtb by only include nodes with compatible strings present in drivers Generate include/generated/compatible.h ./tools/dtoc/dtoc compatible -d u-boot.dtb -o include/generated/compatible.h this will generate compatible.h but the code does not yet support struct udevice_id with data in struct udevice_id and does not filter anything. I will continue working on these features but any early feedback is welcome. To be able to test this a bit more I reworked and back ported the patches and add a bit more of work on top, such as - Add Makefile rules (need to be improved) - Add support for checking enabled drivers for dtb shrink (need to be improved as parsing probably does not take into account no common situations) - Add support for defining constants based on compatible strings enabled This work can be found in https://gitlab.collabora.com/wlozano/u-boot/-/tree/wip The drawback is that this implementation doesn't take advantage of the new abstractions found in the Simon's work, but I think it could still be useful to test the idea, and discuss if it makes sense. I have done some testing in my iMX6 Hummingboard but I have found some issues not related to this work in MMC, which I need to debug. Regards, Walter
Re: [PATCH v4 02/14] dtoc: add missing code comments
Hi Simon, On 25/6/20 22:42, Simon Glass wrote: On Wed, 24 Jun 2020 at 22:10, Walter Lozano wrote: Add missing information about internal class members in order to make the code easier to follow. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 3 +++ 1 file changed, 3 insertions(+) Reviewed-by: Simon Glass BTW if there was a review tag from last version, please add it to your commit. Thanks for pointing that, I'll take you advice for those commit that doesn't change at all. Also thank you for all the time you spent reviewing this series and sharing suggestions. Regards, Walter
Re: [PATCH v4 12/14] arm: dts: include gpio nodes for card detect
Hi Adam, On 26/6/20 09:26, Adam Ford wrote: On Thu, Jun 25, 2020 at 11:37 PM Adam Ford wrote: On Wed, Jun 24, 2020 at 11:11 PM Walter Lozano wrote: Several MMC drivers use GPIO for card detection with cd-gpios property in the MMC node pointing to a GPIO node. However, as U-Boot tries to save space by keeping only required nodes using u-boot* properties, several devices tree result in having only in the MMC node but not the GPIO node associated to cd-gpios. This patch, fixes several ocurrence of this issue. Signed-off-by: Walter Lozano --- arch/arm/dts/da850-evm-u-boot.dtsi| 4 arch/arm/dts/da850-lcdk-u-boot.dtsi | 4 arch/arm/dts/rk3288-u-boot.dtsi | 4 arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi | 2 +- arch/arm/dts/rk3288-veyron-u-boot.dtsi| 11 +++ 5 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 arch/arm/dts/rk3288-veyron-u-boot.dtsi diff --git a/arch/arm/dts/da850-evm-u-boot.dtsi b/arch/arm/dts/da850-evm-u-boot.dtsi index d9afc5edf4..d588628641 100644 --- a/arch/arm/dts/da850-evm-u-boot.dtsi +++ b/arch/arm/dts/da850-evm-u-boot.dtsi @@ -39,3 +39,7 @@ { u-boot,dm-spl; }; + + { + u-boot,dm-spl; +}; I don't know that this is needed for the da850-evm since it doesn't boot from sd/mmc. It can boot from SPI, NAND or NOR Flash depending on the config option selected, but none of them need the gpio during SPL. The gpio is loaded during normal U-Boot. I will try to run some tests to make sure it still boots in the next few days. I know space is getting tight in SPL. I applied your patches and built. FYI, "git am" didn't let them apply nicely, but there could be some missing dependent patches I was missing. I was able to patch with "patch" The board booted as expected. Thanks for your feedback and testing. I examined the generated dtb for SPL since this board doesn't use OF_PLATDATA. It looks like we could remove both the GPIO and the MMC modes from SPL, but I'm not going to worry about it unless we can't boot any more. If/when that happens, I'll spend more time trying to free up space in SPL. For now,, for the series... Tested-by: Adam Ford #da850-evm I found this possible issue while testing different configurations, but it is more related to omapl138_lcdk_defconfig (da850-lcdk) which seems to use OF_PLATDATA. In the case of da850-evm, if there are issue with space in SPL, maybe we can consider removing both MMC and GPIO node and also enable OF_PLATDATA, hopefully if omapl138_lcdk_defconfig works as expected the effort would be little. Regards, Walter diff --git a/arch/arm/dts/da850-lcdk-u-boot.dtsi b/arch/arm/dts/da850-lcdk-u-boot.dtsi index b372d06ca9..d50775c173 100644 --- a/arch/arm/dts/da850-lcdk-u-boot.dtsi +++ b/arch/arm/dts/da850-lcdk-u-boot.dtsi @@ -28,3 +28,7 @@ { u-boot,dm-spl; }; + + { + u-boot,dm-spl; +}; diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi index 6d31735362..51b6e018bd 100644 --- a/arch/arm/dts/rk3288-u-boot.dtsi +++ b/arch/arm/dts/rk3288-u-boot.dtsi @@ -43,3 +43,7 @@ { u-boot,dm-pre-reloc; }; + + { + u-boot,dm-pre-reloc; +}; diff --git a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi index eccc069368..251fbdee71 100644 --- a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi +++ b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi @@ -3,7 +3,7 @@ * Copyright 2015 Google, Inc */ -#include "rk3288-u-boot.dtsi" +#include "rk3288-veyron-u-boot.dtsi" { rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d diff --git a/arch/arm/dts/rk3288-veyron-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-u-boot.dtsi new file mode 100644 index 00..899fe6e7a0 --- /dev/null +++ b/arch/arm/dts/rk3288-veyron-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright 2015 Google, Inc + */ + +#include "rk3288-u-boot.dtsi" + + { + u-boot,dm-pre-reloc; +}; + -- 2.20.1
[PATCH v3 00/14] improve OF_PLATDATA support
When using OF_PLATDATA dtbs are converted to C structs in order to save space as we can remove both dtbs and libraries from TPL/SPL binaries. This patchset tries to improve its support by overcoming some limitations in the current implementation First, the support for scan and check for valid driver/aliases is added in order to generate U_BOOT_DEVICE entries with valid driver names. Secondly, the way information about linked noded (phandle) is generated in C structs is improved in order to make it easier to get a device associated to its data. Lastly the support for the property cd-gpios is added, which is used to configure the card detection gpio on MMC is added. This implementation is based in discussion in [1], [2] and [3] [1] https://patchwork.ozlabs.org/patch/1249198/ [2] https://patchwork.ozlabs.org/project/uboot/list/?series=167495=* [3] https://patchwork.ozlabs.org/project/uboot/list/?series=176759=* Changes for v3 - Split patches to separate core from tools changes - Squash patches to avoid errors on tests - Fix lines longer than 80 chars Changes for v2 - Rename drivers to match compatible strings - Fix out-of-trees usage of dtoc - Fix test coverage - Improve documentation - Improve format Walter Lozano (14): drivers: rename drivers to match compatible string dtoc: add missing code comments core: add support for U_BOOT_DRIVER_ALIAS dtoc: add support to scan drivers dtoc: add option to disable warnings dm: doc: update of-plat with the support for driver aliases core: drop const for struct driver_info core: extend struct driver_info to point to device sandbox: Move section u_boot_list to make it RW dtoc: extend dtoc to use struct driver_info when linking nodes dm: doc: update of-plat with new phandle support arm: dts: include gpio nodes for card detect dtoc: update dtb_platdata to support cd-gpios dtoc: add test for cd-gpios arch/arm/dts/da850-evm-u-boot.dtsi| 4 + arch/arm/dts/da850-lcdk-u-boot.dtsi | 4 + arch/arm/dts/rk3288-u-boot.dtsi | 4 + arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi | 2 +- arch/arm/dts/rk3288-veyron-u-boot.dtsi| 11 + .../mach-at91/arm926ejs/at91sam9260_devices.c | 6 +- .../arm926ejs/at91sam9m10g45_devices.c| 10 +- arch/arm/mach-rockchip/rk3328/syscon_rk3328.c | 4 +- arch/sandbox/cpu/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/omapl138_lcdk.c| 2 +- board/sandbox/sandbox.c | 2 +- doc/driver-model/of-plat.rst | 38 ++- drivers/clk/at91/clk-master.c | 4 +- drivers/clk/at91/clk-peripheral.c | 4 +- drivers/clk/at91/pmc.c| 6 +- drivers/clk/clk-uclass.c | 11 +- drivers/core/device.c | 28 +- drivers/core/root.c | 6 +- drivers/core/simple-bus.c | 4 +- drivers/gpio/at91_gpio.c | 4 +- drivers/gpio/da8xx_gpio.c | 4 +- drivers/gpio/mxs_gpio.c | 8 +- drivers/gpio/rk_gpio.c| 4 +- drivers/gpio/sandbox.c| 6 +- drivers/i2c/rk_i2c.c | 6 +- drivers/input/cros_ec_keyb.c | 4 +- drivers/misc/cros_ec_sandbox.c| 4 +- drivers/misc/irq-uclass.c | 10 +- drivers/mmc/davinci_mmc.c | 4 +- drivers/mmc/ftsdc010_mci.c| 2 +- drivers/mmc/mxsmmc.c | 7 +- drivers/mmc/rockchip_dw_mmc.c | 7 +- drivers/mmc/rockchip_sdhci.c | 2 +- drivers/mtd/spi/sf-uclass.c | 2 +- drivers/mtd/spi/sf_probe.c| 6 +- drivers/pinctrl/nxp/pinctrl-mxs.c | 6 +- drivers/pinctrl/pinctrl-at91.c| 6 +- drivers/pinctrl/rockchip/pinctrl-rk3188.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3288.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3328.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3368.c | 2 +- drivers/power/pmic/rk8xx.c| 6 +- drivers/power/regulator/fixed.c | 4 +- drivers/ram/rockchip/dmc-rk3368.c | 2 +- drivers/ram/rockchip/sdram_rk3188.c | 2 +- drivers/ram/rockchip/sdram_rk3288.c | 2 +- drivers/ram/rockchip/sdram_rk3328.c | 2 +- drivers/ram/rockchip/sdram_rk3399.c | 2 +- drivers/serial/ns16550.c | 4 + drivers/serial/sandbox.c | 6 +- drivers/spi/mxs_spi.c | 8 +- drivers/spi/rk_spi.c | 10 +- drivers/spi/sandbox_spi.c | 4 +- drivers/tpm/tpm_tis_sandbox.c | 4 +- drivers/video/rockchip/rk3288_vop.c | 4 +- drivers/video
[PATCH v4 00/14] improve OF_PLATDATA support
When using OF_PLATDATA dtbs are converted to C structs in order to save space as we can remove both dtbs and libraries from TPL/SPL binaries. This patchset tries to improve its support by overcoming some limitations in the current implementation First, the support for scan and check for valid driver/aliases is added in order to generate U_BOOT_DEVICE entries with valid driver names. Secondly, the way information about linked noded (phandle) is generated in C structs is improved in order to make it easier to get a device associated to its data. Lastly the support for the property cd-gpios is added, which is used to configure the card detection gpio on MMC is added. This implementation is based in discussion in [1], [2] and [3] [1] https://patchwork.ozlabs.org/patch/1249198/ [2] https://patchwork.ozlabs.org/project/uboot/list/?series=167495=* [3] https://patchwork.ozlabs.org/project/uboot/list/?series=176759=* Changes for v4 - Fix syntax error on dtoc Changes for v3 - Split patches to separate core from tools changes - Squash patches to avoid errors on tests - Fix lines longer than 80 chars Changes for v2 - Rename drivers to match compatible strings - Fix out-of-trees usage of dtoc - Fix test coverage - Improve documentation - Improve format Walter Lozano (14): drivers: rename drivers to match compatible string dtoc: add missing code comments core: add support for U_BOOT_DRIVER_ALIAS dtoc: add support to scan drivers dtoc: add option to disable warnings dm: doc: update of-plat with the support for driver aliases core: drop const for struct driver_info core: extend struct driver_info to point to device sandbox: Move section u_boot_list to make it RW dtoc: extend dtoc to use struct driver_info when linking nodes dm: doc: update of-plat with new phandle support arm: dts: include gpio nodes for card detect dtoc: update dtb_platdata to support cd-gpios dtoc: add test for cd-gpios arch/arm/dts/da850-evm-u-boot.dtsi| 4 + arch/arm/dts/da850-lcdk-u-boot.dtsi | 4 + arch/arm/dts/rk3288-u-boot.dtsi | 4 + arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi | 2 +- arch/arm/dts/rk3288-veyron-u-boot.dtsi| 11 + .../mach-at91/arm926ejs/at91sam9260_devices.c | 6 +- .../arm926ejs/at91sam9m10g45_devices.c| 10 +- arch/arm/mach-rockchip/rk3328/syscon_rk3328.c | 4 +- arch/sandbox/cpu/u-boot-spl.lds | 2 +- board/davinci/da8xxevm/omapl138_lcdk.c| 2 +- board/sandbox/sandbox.c | 2 +- doc/driver-model/of-plat.rst | 38 ++- drivers/clk/at91/clk-master.c | 4 +- drivers/clk/at91/clk-peripheral.c | 4 +- drivers/clk/at91/pmc.c| 6 +- drivers/clk/clk-uclass.c | 11 +- drivers/core/device.c | 28 +- drivers/core/root.c | 6 +- drivers/core/simple-bus.c | 4 +- drivers/gpio/at91_gpio.c | 4 +- drivers/gpio/da8xx_gpio.c | 4 +- drivers/gpio/mxs_gpio.c | 8 +- drivers/gpio/rk_gpio.c| 4 +- drivers/gpio/sandbox.c| 6 +- drivers/i2c/rk_i2c.c | 6 +- drivers/input/cros_ec_keyb.c | 4 +- drivers/misc/cros_ec_sandbox.c| 4 +- drivers/misc/irq-uclass.c | 10 +- drivers/mmc/davinci_mmc.c | 4 +- drivers/mmc/ftsdc010_mci.c| 2 +- drivers/mmc/mxsmmc.c | 7 +- drivers/mmc/rockchip_dw_mmc.c | 7 +- drivers/mmc/rockchip_sdhci.c | 2 +- drivers/mtd/spi/sf-uclass.c | 2 +- drivers/mtd/spi/sf_probe.c| 6 +- drivers/pinctrl/nxp/pinctrl-mxs.c | 6 +- drivers/pinctrl/pinctrl-at91.c| 6 +- drivers/pinctrl/rockchip/pinctrl-rk3188.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3288.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3328.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3368.c | 2 +- drivers/power/pmic/rk8xx.c| 6 +- drivers/power/regulator/fixed.c | 4 +- drivers/ram/rockchip/dmc-rk3368.c | 2 +- drivers/ram/rockchip/sdram_rk3188.c | 2 +- drivers/ram/rockchip/sdram_rk3288.c | 2 +- drivers/ram/rockchip/sdram_rk3328.c | 2 +- drivers/ram/rockchip/sdram_rk3399.c | 2 +- drivers/serial/ns16550.c | 4 + drivers/serial/sandbox.c | 6 +- drivers/spi/mxs_spi.c | 8 +- drivers/spi/rk_spi.c | 10 +- drivers/spi/sandbox_spi.c | 4 +- drivers/tpm/tpm_tis_sandbox.c | 4 +- drivers/video/rockchip
[PATCH v4 13/14] dtoc: update dtb_platdata to support cd-gpios
Currently dtoc does not support the property cd-gpios used to declare the gpios for card detect in mmc. This patch adds support to cd-gpios property. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 16 ++-- tools/dtoc/test_dtoc.py| 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index f6579fd655..25ed7f50eb 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -258,7 +258,7 @@ class DtbPlatdata(object): Return: Number of argument cells is this is a phandle, else None """ -if prop.name in ['clocks']: +if prop.name in ['clocks', 'cd-gpios']: if not isinstance(prop.value, list): prop.value = [prop.value] val = prop.value @@ -278,11 +278,14 @@ class DtbPlatdata(object): if not target: raise ValueError("Cannot parse '%s' in node '%s'" % (prop.name, node_name)) -prop_name = '#clock-cells' -cells = target.props.get(prop_name) +cells = None +for prop_name in ['#clock-cells', '#gpio-cells']: +cells = target.props.get(prop_name) +if cells: +break if not cells: -raise ValueError("Node '%s' has no '%s' property" % -(target.name, prop_name)) +raise ValueError("Node '%s' has no cells property" % +(target.name)) num_args = fdt_util.fdt32_to_cpu(cells.value) max_args = max(max_args, num_args) args.append(num_args) @@ -652,7 +655,8 @@ class DtbPlatdata(object): # dtv_dmc_at_xxx.clocks[0].node = DM_GET_DEVICE(clock_controller_at_xxx) self.buf('void dm_populate_phandle_data(void) {\n') for l in self._links: -self.buf('\t%s = DM_GET_DEVICE(%s);\n' % (l['var_node'], l['dev_name'])) +self.buf('\t%s = DM_GET_DEVICE(%s);\n' % + (l['var_node'], l['dev_name'])) self.buf('}\n') self.out(''.join(self.get_buf())) diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 209542c849..67ca9a8da1 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -485,7 +485,7 @@ void dm_populate_phandle_data(void) { output = tools.GetOutputFilename('output') with self.assertRaises(ValueError) as e: self.run_test(['struct'], dtb_file, output) -self.assertIn("Node 'phandle-target' has no '#clock-cells' property", +self.assertIn("Node 'phandle-target' has no cells property", str(e.exception)) def test_aliases(self): -- 2.20.1
[PATCH v4 10/14] dtoc: extend dtoc to use struct driver_info when linking nodes
In the current implementation, when dtoc parses a dtb to generate a struct platdata it converts the information related to linked nodes as pointers to struct platdata of destination nodes. By doing this, it makes difficult to get pointer to udevices created based on these information. This patch extends dtoc to use struct driver_info when populating information about linked nodes, which makes it easier to later get the devices created. In this context, reimplement functions like clk_get_by_index_platdata() which made use of the previous approach. Signed-off-by: Walter Lozano --- drivers/clk/clk-uclass.c| 11 ++- drivers/misc/irq-uclass.c | 10 ++- drivers/mmc/ftsdc010_mci.c | 2 +- drivers/mmc/rockchip_dw_mmc.c | 2 +- drivers/mmc/rockchip_sdhci.c| 2 +- drivers/ram/rockchip/sdram_rk3399.c | 2 +- drivers/spi/rk_spi.c| 2 +- include/clk.h | 4 +- tools/dtoc/dtb_platdata.py | 26 ++- tools/dtoc/test_dtoc.py | 104 10 files changed, 100 insertions(+), 65 deletions(-) diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 71878474eb..412f26cd29 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -25,17 +25,16 @@ static inline const struct clk_ops *clk_dev_ops(struct udevice *dev) #if CONFIG_IS_ENABLED(OF_CONTROL) # if CONFIG_IS_ENABLED(OF_PLATDATA) -int clk_get_by_index_platdata(struct udevice *dev, int index, - struct phandle_1_arg *cells, struct clk *clk) +int clk_get_by_driver_info(struct udevice *dev, struct phandle_1_arg *cells, + struct clk *clk) { int ret; - if (index != 0) - return -ENOSYS; - ret = uclass_get_device(UCLASS_CLK, 0, >dev); + ret = device_get_by_driver_info((struct driver_info *)cells->node, + >dev); if (ret) return ret; - clk->id = cells[0].arg[0]; + clk->id = cells->arg[0]; return 0; } diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c index 61aa10e465..3c38681108 100644 --- a/drivers/misc/irq-uclass.c +++ b/drivers/misc/irq-uclass.c @@ -63,17 +63,15 @@ int irq_read_and_clear(struct irq *irq) } #if CONFIG_IS_ENABLED(OF_PLATDATA) -int irq_get_by_index_platdata(struct udevice *dev, int index, - struct phandle_1_arg *cells, struct irq *irq) +int irq_get_by_driver_info(struct udevice *dev, + struct phandle_1_arg *cells, struct irq *irq) { int ret; - if (index != 0) - return -ENOSYS; - ret = uclass_get_device(UCLASS_IRQ, 0, >dev); + ret = device_get_by_driver_info(cells->node, >dev); if (ret) return ret; - irq->id = cells[0].arg[0]; + irq->id = cells->arg[0]; return 0; } diff --git a/drivers/mmc/ftsdc010_mci.c b/drivers/mmc/ftsdc010_mci.c index 9c15eb36d6..efa92d48be 100644 --- a/drivers/mmc/ftsdc010_mci.c +++ b/drivers/mmc/ftsdc010_mci.c @@ -437,7 +437,7 @@ static int ftsdc010_mmc_probe(struct udevice *dev) chip->priv = dev; chip->dev_index = 1; memcpy(priv->minmax, dtplat->clock_freq_min_max, sizeof(priv->minmax)); - ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, >clk); + ret = clk_get_by_driver_info(dev, dtplat->clocks, >clk); if (ret < 0) return ret; #endif diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index ac710324c8..80432ddbbc 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -120,7 +120,7 @@ static int rockchip_dwmmc_probe(struct udevice *dev) priv->minmax[0] = 40; /* 400 kHz */ priv->minmax[1] = dtplat->max_frequency; - ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, >clk); + ret = clk_get_by_driver_info(dev, dtplat->clocks, >clk); if (ret < 0) return ret; #else diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index b440996b26..b073f1a08d 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -46,7 +46,7 @@ static int arasan_sdhci_probe(struct udevice *dev) host->name = dev->name; host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]); max_frequency = dtplat->max_frequency; - ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, ); + ret = clk_get_by_driver_info(dev, dtplat->clocks, ); #else max_frequency = dev_read_u32_default(dev, "max-frequency", 0); ret = clk_get_by_index(dev, 0, ); diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index d69ef01d08..87ec25f893 100644 --- a/drivers/r
[PATCH v4 12/14] arm: dts: include gpio nodes for card detect
Several MMC drivers use GPIO for card detection with cd-gpios property in the MMC node pointing to a GPIO node. However, as U-Boot tries to save space by keeping only required nodes using u-boot* properties, several devices tree result in having only in the MMC node but not the GPIO node associated to cd-gpios. This patch, fixes several ocurrence of this issue. Signed-off-by: Walter Lozano --- arch/arm/dts/da850-evm-u-boot.dtsi| 4 arch/arm/dts/da850-lcdk-u-boot.dtsi | 4 arch/arm/dts/rk3288-u-boot.dtsi | 4 arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi | 2 +- arch/arm/dts/rk3288-veyron-u-boot.dtsi| 11 +++ 5 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 arch/arm/dts/rk3288-veyron-u-boot.dtsi diff --git a/arch/arm/dts/da850-evm-u-boot.dtsi b/arch/arm/dts/da850-evm-u-boot.dtsi index d9afc5edf4..d588628641 100644 --- a/arch/arm/dts/da850-evm-u-boot.dtsi +++ b/arch/arm/dts/da850-evm-u-boot.dtsi @@ -39,3 +39,7 @@ { u-boot,dm-spl; }; + + { + u-boot,dm-spl; +}; diff --git a/arch/arm/dts/da850-lcdk-u-boot.dtsi b/arch/arm/dts/da850-lcdk-u-boot.dtsi index b372d06ca9..d50775c173 100644 --- a/arch/arm/dts/da850-lcdk-u-boot.dtsi +++ b/arch/arm/dts/da850-lcdk-u-boot.dtsi @@ -28,3 +28,7 @@ { u-boot,dm-spl; }; + + { + u-boot,dm-spl; +}; diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi index 6d31735362..51b6e018bd 100644 --- a/arch/arm/dts/rk3288-u-boot.dtsi +++ b/arch/arm/dts/rk3288-u-boot.dtsi @@ -43,3 +43,7 @@ { u-boot,dm-pre-reloc; }; + + { + u-boot,dm-pre-reloc; +}; diff --git a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi index eccc069368..251fbdee71 100644 --- a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi +++ b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi @@ -3,7 +3,7 @@ * Copyright 2015 Google, Inc */ -#include "rk3288-u-boot.dtsi" +#include "rk3288-veyron-u-boot.dtsi" { rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d diff --git a/arch/arm/dts/rk3288-veyron-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-u-boot.dtsi new file mode 100644 index 00..899fe6e7a0 --- /dev/null +++ b/arch/arm/dts/rk3288-veyron-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright 2015 Google, Inc + */ + +#include "rk3288-u-boot.dtsi" + + { + u-boot,dm-pre-reloc; +}; + -- 2.20.1
[PATCH v4 11/14] dm: doc: update of-plat with new phandle support
Update documentation to reflect the new phandle support when OF_PLATDATA is used. Now phandles are implemented as pointers to U_BOOT_DEVICE, which makes it possible to get a pointer to the actual device. Signed-off-by: Walter Lozano --- doc/driver-model/of-plat.rst | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/doc/driver-model/of-plat.rst b/doc/driver-model/of-plat.rst index 376d4409a5..1e3fad137b 100644 --- a/doc/driver-model/of-plat.rst +++ b/doc/driver-model/of-plat.rst @@ -69,9 +69,8 @@ strictly necessary. Notable problems include: - Correct relations between nodes are not implemented. This means that parent/child relations (like bus device iteration) do not work yet. Some phandles (those that are recognised as such) are converted into - a pointer to platform data. This pointer can potentially be used to - access the referenced device (by searching for the pointer value). - This feature is not yet implemented, however. + a pointer to struct driver_info. This pointer can be used to access + the referenced device. How it works @@ -146,10 +145,10 @@ and the following device declaration: .clock_freq_min_max = {0x61a80, 0x8f0d180}, .vmmc_supply= 0xb, .num_slots = 0x1, -.clocks = {{_clock_controller_at_ff76, 456}, - {_clock_controller_at_ff76, 68}, - {_clock_controller_at_ff76, 114}, - {_clock_controller_at_ff76, 118}}, +.clocks = {{NULL, 456}, + {NULL, 68}, + {NULL, 114}, + {NULL, 118}}, .cap_mmc_highspeed = true, .disable_wp = true, .bus_width = 0x4, @@ -164,6 +163,13 @@ and the following device declaration: .platdata_size = sizeof(dtv_dwmmc_at_ff0c), }; +void dm_populate_phandle_data(void) { +dtv_dwmmc_at_ff0c.clocks[0].node = DM_GET_DEVICE(clock_controller_at_ff76); +dtv_dwmmc_at_ff0c.clocks[1].node = DM_GET_DEVICE(clock_controller_at_ff76); +dtv_dwmmc_at_ff0c.clocks[2].node = DM_GET_DEVICE(clock_controller_at_ff76); +dtv_dwmmc_at_ff0c.clocks[3].node = DM_GET_DEVICE(clock_controller_at_ff76); +} + The device is then instantiated at run-time and the platform data can be accessed using: @@ -329,7 +335,9 @@ prevents them being used inadvertently. All usage must be bracketed with #if CONFIG_IS_ENABLED(OF_PLATDATA). The dt-platdata.c file contains the device declarations and is is built in -spl/dt-platdata.c. +spl/dt-platdata.c. It additionally contains the definition of +dm_populate_phandle_data() which is responsible of filling the phandle +information by adding references to U_BOOT_DEVICE by using DM_GET_DEVICE The beginnings of a libfdt Python module are provided. So far this only implements a subset of the features. -- 2.20.1
[PATCH v4 09/14] sandbox: Move section u_boot_list to make it RW
In order to be able to update data in u_boot_list, move this section to make it RW. Signed-off-by: Walter Lozano --- arch/sandbox/cpu/u-boot-spl.lds | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/sandbox/cpu/u-boot-spl.lds b/arch/sandbox/cpu/u-boot-spl.lds index de65b01b33..c60eb109b1 100644 --- a/arch/sandbox/cpu/u-boot-spl.lds +++ b/arch/sandbox/cpu/u-boot-spl.lds @@ -20,4 +20,4 @@ SECTIONS __bss_start = .; } -INSERT BEFORE .data; +INSERT AFTER .data; -- 2.20.1
[PATCH v4 14/14] dtoc: add test for cd-gpios
Add a test for dtoc taking into account the cd-gpios property. Signed-off-by: Walter Lozano --- tools/dtoc/dtoc_test_phandle_cd_gpios.dts | 42 ++ tools/dtoc/test_dtoc.py | 67 +++ 2 files changed, 109 insertions(+) create mode 100644 tools/dtoc/dtoc_test_phandle_cd_gpios.dts diff --git a/tools/dtoc/dtoc_test_phandle_cd_gpios.dts b/tools/dtoc/dtoc_test_phandle_cd_gpios.dts new file mode 100644 index 00..241743e73e --- /dev/null +++ b/tools/dtoc/dtoc_test_phandle_cd_gpios.dts @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2020 Collabora Ltd. + */ + +/dts-v1/; + +/ { + phandle: phandle-target { + u-boot,dm-pre-reloc; + compatible = "target"; + intval = <0>; + #gpio-cells = <0>; + }; + + phandle_1: phandle2-target { + u-boot,dm-pre-reloc; + compatible = "target"; + intval = <1>; + #gpio-cells = <1>; + }; + phandle_2: phandle3-target { + u-boot,dm-pre-reloc; + compatible = "target"; + intval = <2>; + #gpio-cells = <2>; + }; + + phandle-source { + u-boot,dm-pre-reloc; + compatible = "source"; + cd-gpios = < _1 11 _2 12 13 >; + }; + + phandle-source2 { + u-boot,dm-pre-reloc; + compatible = "source"; + cd-gpios = <>; + }; +}; diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 67ca9a8da1..3c8e343b1f 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -466,6 +466,73 @@ U_BOOT_DEVICE(phandle_source2) = { void dm_populate_phandle_data(void) { \tdtv_phandle_source2.clocks[0].node = DM_GET_DEVICE(phandle_target); } +''', data) + +def test_phandle_cd_gpio(self): +"""Test that phandle targets are generated when unsing cd-gpios""" +dtb_file = get_dtb_file('dtoc_test_phandle_cd_gpios.dts') +output = tools.GetOutputFilename('output') +dtb_platdata.run_steps(['platdata'], dtb_file, False, output, True) +with open(output) as infile: +data = infile.read() +self._CheckStrings(C_HEADER + ''' +static struct dtd_target dtv_phandle_target = { +\t.intval\t\t\t= 0x0, +}; +U_BOOT_DEVICE(phandle_target) = { +\t.name\t\t= "target", +\t.platdata\t= _phandle_target, +\t.platdata_size\t= sizeof(dtv_phandle_target), +}; + +static struct dtd_target dtv_phandle2_target = { +\t.intval\t\t\t= 0x1, +}; +U_BOOT_DEVICE(phandle2_target) = { +\t.name\t\t= "target", +\t.platdata\t= _phandle2_target, +\t.platdata_size\t= sizeof(dtv_phandle2_target), +}; + +static struct dtd_target dtv_phandle3_target = { +\t.intval\t\t\t= 0x2, +}; +U_BOOT_DEVICE(phandle3_target) = { +\t.name\t\t= "target", +\t.platdata\t= _phandle3_target, +\t.platdata_size\t= sizeof(dtv_phandle3_target), +}; + +static struct dtd_source dtv_phandle_source = { +\t.cd_gpios\t\t= { +\t\t\t{NULL, {}}, +\t\t\t{NULL, {11}}, +\t\t\t{NULL, {12, 13}}, +\t\t\t{NULL, {}},}, +}; +U_BOOT_DEVICE(phandle_source) = { +\t.name\t\t= "source", +\t.platdata\t= _phandle_source, +\t.platdata_size\t= sizeof(dtv_phandle_source), +}; + +static struct dtd_source dtv_phandle_source2 = { +\t.cd_gpios\t\t= { +\t\t\t{NULL, {}},}, +}; +U_BOOT_DEVICE(phandle_source2) = { +\t.name\t\t= "source", +\t.platdata\t= _phandle_source2, +\t.platdata_size\t= sizeof(dtv_phandle_source2), +}; + +void dm_populate_phandle_data(void) { +\tdtv_phandle_source.cd_gpios[0].node = DM_GET_DEVICE(phandle_target); +\tdtv_phandle_source.cd_gpios[1].node = DM_GET_DEVICE(phandle2_target); +\tdtv_phandle_source.cd_gpios[2].node = DM_GET_DEVICE(phandle3_target); +\tdtv_phandle_source.cd_gpios[3].node = DM_GET_DEVICE(phandle_target); +\tdtv_phandle_source2.cd_gpios[0].node = DM_GET_DEVICE(phandle_target); +} ''', data) def test_phandle_bad(self): -- 2.20.1
[PATCH v4 08/14] core: extend struct driver_info to point to device
Currently when creating an U_BOOT_DEVICE entry a struct driver_info is declared, which contains the data needed to instantiate the device. However, the actual device is created at runtime and there is no proper way to get the device based on its struct driver_info. This patch extends struct driver_info adding a pointer to udevice which is populated during the bind process, allowing to generate a set of functions to get the device based on its struct driver_info. Signed-off-by: Walter Lozano --- drivers/core/device.c | 26 +++--- drivers/core/root.c | 4 include/dm/device.h | 15 +++ include/dm/platdata.h | 14 ++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/drivers/core/device.c b/drivers/core/device.c index a0ad080aaf..4f8c97a195 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -250,6 +250,7 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, { struct driver *drv; uint platdata_size = 0; + int ret; drv = lists_driver_lookup_name(info->name); if (!drv) @@ -260,9 +261,16 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, #if CONFIG_IS_ENABLED(OF_PLATDATA) platdata_size = info->platdata_size; #endif - return device_bind_common(parent, drv, info->name, - (void *)info->platdata, 0, ofnode_null(), platdata_size, - devp); + ret = device_bind_common(parent, drv, info->name, +(void *)info->platdata, 0, ofnode_null(), +platdata_size, devp); + if (ret) + return ret; +#if CONFIG_IS_ENABLED(OF_PLATDATA) + info->dev = *devp; +#endif + + return ret; } static void *alloc_priv(int size, uint flags) @@ -727,6 +735,18 @@ int device_get_global_by_ofnode(ofnode ofnode, struct udevice **devp) return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp); } +#if CONFIG_IS_ENABLED(OF_PLATDATA) +int device_get_by_driver_info(const struct driver_info *info, + struct udevice **devp) +{ + struct udevice *dev; + + dev = info->dev; + + return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp); +} +#endif + int device_find_first_child(const struct udevice *parent, struct udevice **devp) { if (list_empty(>child_head)) { diff --git a/drivers/core/root.c b/drivers/core/root.c index c9ee56478a..2643ef68a7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -346,6 +346,10 @@ int dm_init_and_scan(bool pre_reloc_only) { int ret; +#if CONFIG_IS_ENABLED(OF_PLATDATA) + dm_populate_phandle_data(); +#endif + ret = dm_init(IS_ENABLED(CONFIG_OF_LIVE)); if (ret) { debug("dm_init() failed: %d\n", ret); diff --git a/include/dm/device.h b/include/dm/device.h index 2cfe10766f..f5738a0cee 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -538,6 +538,21 @@ int device_find_global_by_ofnode(ofnode node, struct udevice **devp); */ int device_get_global_by_ofnode(ofnode node, struct udevice **devp); +/** + * device_get_by_driver_info() - Get a device based on driver_info + * + * Locates a device by its struct driver_info, by using its reference which + * is updated during the bind process. + * + * The device is probed to activate it ready for use. + * + * @info: Struct driver_info + * @devp: Returns pointer to device if found, otherwise this is set to NULL + * @return 0 if OK, -ve on error + */ +int device_get_by_driver_info(const struct driver_info *info, + struct udevice **devp); + /** * device_find_first_child() - Find the first child of a device * diff --git a/include/dm/platdata.h b/include/dm/platdata.h index c972fa6936..cab93b071b 100644 --- a/include/dm/platdata.h +++ b/include/dm/platdata.h @@ -22,12 +22,14 @@ * @name: Driver name * @platdata: Driver-specific platform data * @platdata_size: Size of platform data structure + * @dev: Device created from this structure data */ struct driver_info { const char *name; const void *platdata; #if CONFIG_IS_ENABLED(OF_PLATDATA) uint platdata_size; + struct udevice *dev; #endif }; @@ -43,4 +45,16 @@ struct driver_info { #define U_BOOT_DEVICES(__name) \ ll_entry_declare_list(struct driver_info, __name, driver_info) +/* Get a pointer to a given driver */ +#define DM_GET_DEVICE(__name) \ + ll_entry_get(struct driver_info, __name, driver_info) + +/** + * dm_populate_phandle_data() - Populates phandle data in platda + * + * This populates phandle data with an U_BOOT_DEVICE entry get by + * DM_GET_DEVICE. The implementation of this function will be done + * by dtoc when parsing dtb. + */ +void dm_populate_phandle_data(void); #endif -- 2.20.1
[PATCH v4 03/14] core: add support for U_BOOT_DRIVER_ALIAS
Currently when using OF_PLATDATA the binding between devices and drivers is done trying to match the compatible string in the node with a driver name. However, usually a single driver supports multiple compatible strings which causes that only devices which its compatible string matches a driver name get bound. To overcome this issue, this patch adds the U_BOOT_DRIVER_ALIAS macro, which generates no code at all, but allows an easy way to declare driver name aliases. Thanks to this, dtoc could be improve to look for the driver name based on its alias when it populates the U_BOOT_DEVICE entry. Signed-off-by: Walter Lozano --- drivers/clk/at91/pmc.c| 2 ++ drivers/gpio/mxs_gpio.c | 2 ++ drivers/gpio/sandbox.c| 2 ++ drivers/i2c/rk_i2c.c | 2 ++ drivers/mmc/mxsmmc.c | 1 + drivers/mmc/rockchip_dw_mmc.c | 3 +++ drivers/mtd/spi/sf_probe.c| 2 ++ drivers/pinctrl/nxp/pinctrl-mxs.c | 2 ++ drivers/pinctrl/pinctrl-at91.c| 2 ++ drivers/power/pmic/rk8xx.c| 2 ++ drivers/serial/ns16550.c | 4 drivers/spi/mxs_spi.c | 2 ++ drivers/spi/rk_spi.c | 2 ++ include/dm/device.h | 7 +++ 14 files changed, 35 insertions(+) diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c index 1fede16a0c..793a506d27 100644 --- a/drivers/clk/at91/pmc.c +++ b/drivers/clk/at91/pmc.c @@ -30,6 +30,8 @@ U_BOOT_DRIVER(atmel_at91rm9200_pmc) = { .of_match = at91_pmc_match, }; +U_BOOT_DRIVER_ALIAS(atmel_at91rm9200_pmc, atmel_at91sam9260_pmc) + /*-*/ int at91_pmc_core_probe(struct udevice *dev) diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c index e43484d13a..bcdf08c255 100644 --- a/drivers/gpio/mxs_gpio.c +++ b/drivers/gpio/mxs_gpio.c @@ -309,4 +309,6 @@ U_BOOT_DRIVER(fsl_imx23_gpio) = { .ofdata_to_platdata = mxs_ofdata_to_platdata, #endif }; + +U_BOOT_DRIVER_ALIAS(fsl_imx23_gpio, fsl_imx28_gpio) #endif /* DM_GPIO */ diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index ff46d3c8d1..8923e54867 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -253,6 +253,8 @@ U_BOOT_DRIVER(sandbox_gpio) = { .ops= _sandbox_ops, }; +U_BOOT_DRIVER_ALIAS(sandbox_gpio, sandbox_gpio_alias) + /* pincontrol: used only to check GPIO pin configuration (pinmux command) */ struct sb_pinctrl_priv { diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c index eceef80e70..e76c087b1d 100644 --- a/drivers/i2c/rk_i2c.c +++ b/drivers/i2c/rk_i2c.c @@ -492,3 +492,5 @@ U_BOOT_DRIVER(rockchip_rk3066_i2c) = { .priv_auto_alloc_size = sizeof(struct rk_i2c), .ops= _i2c_ops, }; + +U_BOOT_DRIVER_ALIAS(rockchip_rk3066_i2c, rockchip_rk3288_i2c) diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c index 35c336b499..afa95e57ee 100644 --- a/drivers/mmc/mxsmmc.c +++ b/drivers/mmc/mxsmmc.c @@ -724,4 +724,5 @@ U_BOOT_DRIVER(fsl_imx23_mmc) = { .platdata_auto_alloc_size = sizeof(struct mxsmmc_platdata), }; +U_BOOT_DRIVER_ALIAS(fsl_imx23_mmc, fsl_imx28_mmc) #endif /* CONFIG_DM_MMC */ diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index ef75367b3e..ac710324c8 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -178,6 +178,9 @@ U_BOOT_DRIVER(rockchip_rk3288_dw_mshc) = { .platdata_auto_alloc_size = sizeof(struct rockchip_mmc_plat), }; +U_BOOT_DRIVER_ALIAS(rockchip_rk3288_dw_mshc, rockchip_rk3328_dw_mshc) +U_BOOT_DRIVER_ALIAS(rockchip_rk3288_dw_mshc, rockchip_rk3368_dw_mshc) + #ifdef CONFIG_PWRSEQ static int rockchip_dwmmc_pwrseq_set_power(struct udevice *dev, bool enable) { diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index 1b44cc68c6..97fa22a4b3 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -180,4 +180,6 @@ U_BOOT_DRIVER(jedec_spi_nor) = { .ops= _flash_std_ops, }; +U_BOOT_DRIVER_ALIAS(jedec_spi_nor, spansion_m25p16) + #endif /* CONFIG_DM_SPI_FLASH */ diff --git a/drivers/pinctrl/nxp/pinctrl-mxs.c b/drivers/pinctrl/nxp/pinctrl-mxs.c index bd434667b1..da6b95acc5 100644 --- a/drivers/pinctrl/nxp/pinctrl-mxs.c +++ b/drivers/pinctrl/nxp/pinctrl-mxs.c @@ -190,3 +190,5 @@ U_BOOT_DRIVER(fsl_imx23_pinctrl) = { .priv_auto_alloc_size = sizeof(struct mxs_pinctrl_priv), .ops = _pinctrl_ops, }; + +U_BOOT_DRIVER_ALIAS(fsl_imx23_pinctrl, fsl_imx28_pinctrl) diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c index 0cc35042f5..b40ff8c823 100644 --- a/drivers/pinctrl/pinctrl-at91.c +++ b/drivers/pinctrl/pinctrl-at91.c @@ -525,3 +525,5 @@ U_BOOT_DRIVER(atmel_sama5d3_pinctrl) = { .priv_auto_alloc_size = sizeof(struct at91_pinctrl_priv), .ops = _pinctrl_ops, }; + +U_BOOT_DRIVER_ALIAS(atmel_sama5d3_pinctrl, atmel_at91rm9200_pinctrl) diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power
[PATCH v4 07/14] core: drop const for struct driver_info
In order to prepare for a new support of phandle when OF_PLATDATA is used drop the const for struct driver_info as this struct will need to be updated on runtime. Signed-off-by: Walter Lozano --- drivers/core/device.c| 2 +- drivers/core/root.c | 2 +- include/dm/device-internal.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/core/device.c b/drivers/core/device.c index 0157bb1fe0..a0ad080aaf 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -246,7 +246,7 @@ int device_bind_ofnode(struct udevice *parent, const struct driver *drv, } int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, - const struct driver_info *info, struct udevice **devp) + struct driver_info *info, struct udevice **devp) { struct driver *drv; uint platdata_size = 0; diff --git a/drivers/core/root.c b/drivers/core/root.c index 14df16c280..c9ee56478a 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -25,7 +25,7 @@ DECLARE_GLOBAL_DATA_PTR; -static const struct driver_info root_info = { +static struct driver_info root_info = { .name = "root_driver", }; diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index 294d6c1810..5145fb4e14 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -81,7 +81,7 @@ int device_bind_with_driver_data(struct udevice *parent, * @return 0 if OK, -ve on error */ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, - const struct driver_info *info, struct udevice **devp); + struct driver_info *info, struct udevice **devp); /** * device_ofdata_to_platdata() - Read platform data for a device -- 2.20.1
[PATCH v4 04/14] dtoc: add support to scan drivers
Currently dtoc scans dtbs to convert them to struct platdata and to generate U_BOOT_DEVICE entries. These entries need to be filled with the driver name, but at this moment the information used is the compatible name present in the dtb. This causes that only nodes with a compatible name that matches a driver name generate a working entry. In order to improve this behaviour, this patch adds to dtoc the capability of scan drivers source code to generate a list of valid driver names and aliases. This allows to generate U_BOOT_DEVICE entries using valid driver names and rise a warning in the case a name is not valid. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 91 -- tools/dtoc/test_dtoc.py| 33 ++ 2 files changed, 120 insertions(+), 4 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index bc0de426a9..ae8674d85c 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -13,6 +13,8 @@ static data. import collections import copy +import os +import re import sys from dtoc import fdt @@ -143,6 +145,11 @@ class DtbPlatdata(object): _aliases: Dict that hold aliases for compatible strings key: First compatible string declared in a node value: List of additional compatible strings declared in a node +_drivers: List of valid driver names found in drivers/ +_driver_aliases: Dict that holds aliases for driver names +key: Driver alias declared with +U_BOOT_DRIVER_ALIAS(driver_alias, driver_name) +value: Driver name declared with U_BOOT_DRIVER(driver_name) """ def __init__(self, dtb_fname, include_disabled): self._fdt = None @@ -152,6 +159,38 @@ class DtbPlatdata(object): self._outfile = None self._lines = [] self._aliases = {} +self._drivers = [] +self._driver_aliases = {} + +def get_normalized_compat_name(self, node): +"""Get a node's normalized compat name + +Returns a valid driver name by retrieving node's first compatible +string as a C identifier and performing a check against _drivers +and a lookup in driver_aliases printing a warning in case of failure. + +Args: +node: Node object to check +Return: +Tuple: +Driver name associated with the first compatible string +List of C identifiers for all the other compatible strings +(possibly empty) +In case of no match found, the return will be the same as +get_compat_name() +""" +compat_c, aliases_c = get_compat_name(node) +if compat_c not in self._drivers: +compat_c_old = compat_c +compat_c = self._driver_aliases.get(compat_c) +if not compat_c: +print('WARNING: the driver %s was not found in the driver list' + % (compat_c_old)) +compat_c = compat_c_old +else: +aliases_c = [compat_c_old] + aliases_c + +return compat_c, aliases_c def setup_output(self, fname): """Set up the output destination @@ -246,6 +285,49 @@ class DtbPlatdata(object): return PhandleInfo(max_args, args) return None +def scan_driver(self, fn): +"""Scan a driver file to build a list of driver names and aliases + +This procedure will populate self._drivers and self._driver_aliases + +Args +fn: Driver filename to scan +""" +with open(fn) as fd: +buff = fd.read() + +# The following re will search for driver names declared as +# U_BOOT_DRIVER(driver_name) +drivers = re.findall('U_BOOT_DRIVER\((.*)\)', buff) + +for driver in drivers: +self._drivers.append(driver) + +# The following re will search for driver aliases declared as +# U_BOOT_DRIVER_ALIAS(alias, driver_name) +driver_aliases = re.findall('U_BOOT_DRIVER_ALIAS\(\s*(\w+)\s*,\s*(\w+)\s*\)', +buff) + +for alias in driver_aliases: # pragma: no cover +if len(alias) != 2: +continue +self._driver_aliases[alias[1]] = alias[0] + +def scan_drivers(self): +"""Scan the driver folders to build a list of driver names and aliases + +This procedure will populate self._drivers and self._driver_aliases + +""" +basedir = sys.argv[0].replace('tools/dtoc/dtoc', '') +if basedir == '': +basedir = './' +for (dirpath, dirnames, filenames) in os.walk(basedir): +for fn in filenames: +
[PATCH v4 05/14] dtoc: add option to disable warnings
As dtoc now performs checks for valid driver names, when running dtoc tests several warnings arise as these tests don't use valid driver names. This patch adds an option to disable those warning, which is only intended for running tests. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 13 ++-- tools/dtoc/dtoc_test_invalid_driver.dts | 15 tools/dtoc/test_dtoc.py | 91 + 3 files changed, 85 insertions(+), 34 deletions(-) create mode 100644 tools/dtoc/dtoc_test_invalid_driver.dts diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index ae8674d85c..d0cd4bf9bd 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -141,6 +141,7 @@ class DtbPlatdata(object): _valid_nodes: A list of Node object with compatible strings _include_disabled: true to include nodes marked status = "disabled" _outfile: The current output file (sys.stdout or a real file) +_warning_disabled: true to disable warnings about driver names not found _lines: Stashed list of output lines for outputting in the future _aliases: Dict that hold aliases for compatible strings key: First compatible string declared in a node @@ -151,12 +152,13 @@ class DtbPlatdata(object): U_BOOT_DRIVER_ALIAS(driver_alias, driver_name) value: Driver name declared with U_BOOT_DRIVER(driver_name) """ -def __init__(self, dtb_fname, include_disabled): +def __init__(self, dtb_fname, include_disabled, warning_disabled): self._fdt = None self._dtb_fname = dtb_fname self._valid_nodes = None self._include_disabled = include_disabled self._outfile = None +self._warning_disabled = warning_disabled self._lines = [] self._aliases = {} self._drivers = [] @@ -184,8 +186,9 @@ class DtbPlatdata(object): compat_c_old = compat_c compat_c = self._driver_aliases.get(compat_c) if not compat_c: -print('WARNING: the driver %s was not found in the driver list' - % (compat_c_old)) +if not self._warning_disabled: +print('WARNING: the driver %s was not found in the driver list' + % (compat_c_old)) compat_c = compat_c_old else: aliases_c = [compat_c_old] + aliases_c @@ -634,7 +637,7 @@ class DtbPlatdata(object): nodes_to_output.remove(node) -def run_steps(args, dtb_file, include_disabled, output): +def run_steps(args, dtb_file, include_disabled, output, warning_disabled=False): """Run all the steps of the dtoc tool Args: @@ -646,7 +649,7 @@ def run_steps(args, dtb_file, include_disabled, output): if not args: raise ValueError('Please specify a command: struct, platdata') -plat = DtbPlatdata(dtb_file, include_disabled) +plat = DtbPlatdata(dtb_file, include_disabled, warning_disabled) plat.scan_drivers() plat.scan_dtb() plat.scan_tree() diff --git a/tools/dtoc/dtoc_test_invalid_driver.dts b/tools/dtoc/dtoc_test_invalid_driver.dts new file mode 100644 index 00..914ac3e899 --- /dev/null +++ b/tools/dtoc/dtoc_test_invalid_driver.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2017 Google, Inc + */ + +/dts-v1/; + +/ { + spl-test { + u-boot,dm-pre-reloc; + compatible = "invalid"; + }; +}; diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 91fc9d77f3..ae3ec509c1 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -12,6 +12,7 @@ tool. import collections import os import struct +import sys import unittest from dtoc import dtb_platdata @@ -101,6 +102,10 @@ class TestDtoc(unittest.TestCase): print('Failures written to /tmp/binman.{expected,actual}') self.assertEquals(expected, actual) + +def run_test(self, args, dtb_file, output): +dtb_platdata.run_steps(args, dtb_file, False, output, True) + def test_name(self): """Test conversion of device tree names to C identifiers""" self.assertEqual('serial_at_0x12', conv_name_to_c('serial@0x12')) @@ -154,12 +159,12 @@ class TestDtoc(unittest.TestCase): """Test output from a device tree file with no nodes""" dtb_file = get_dtb_file('dtoc_test_empty.dts') output = tools.GetOutputFilename('output') -dtb_platdata.run_steps(['struct'], dtb_file, False, output) +self.run_test(['struct'], dtb_file, output) with open(output) as infile: lines = infile.read().splitlines() self.assertEqual(HEADER.splitlines(), lines) -
[PATCH v4 06/14] dm: doc: update of-plat with the support for driver aliases
Update the documentation with the support for driver aliases using U_BOOT_DRIVER_ALIAS. Signed-off-by: Walter Lozano --- doc/driver-model/of-plat.rst | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/doc/driver-model/of-plat.rst b/doc/driver-model/of-plat.rst index 034a68bb4e..376d4409a5 100644 --- a/doc/driver-model/of-plat.rst +++ b/doc/driver-model/of-plat.rst @@ -183,6 +183,17 @@ via U_BOOT_DRIVER(). This effectively means that a U_BOOT_DRIVER() with a it to a valid name for C) is needed, so a dedicated driver is required for each 'compatible' string. +In order to make this a bit more flexible U_BOOT_DRIVER_ALIAS macro can be +used to declare an alias for a driver name, typically a 'compatible' string. +This macro produces no code, but it is by dtoc tool. + +During the build process dtoc parses both U_BOOT_DRIVER and U_BOOT_DRIVER_ALIAS +to build a list of valid driver names and driver aliases. If the 'compatible' +string used for a device does not not match a valid driver name, it will be +checked against the list of driver aliases in order to get the right driver +name to use. If in this step there is no match found a warning is issued to +avoid run-time failures. + Where a node has multiple compatible strings, a #define is used to make them equivalent, e.g.: @@ -269,7 +280,7 @@ For example: }; U_BOOT_DRIVER(mmc_drv) = { -.name = "vendor_mmc", /* matches compatible string */ +.name = "mmc_drv", .id = UCLASS_MMC, .of_match = mmc_ids, .ofdata_to_platdata = mmc_ofdata_to_platdata, @@ -278,6 +289,7 @@ For example: .platdata_auto_alloc_size = sizeof(struct mmc_platdata), }; +U_BOOT_DRIVER_ALIAS(mmc_drv, vendor_mmc) /* matches compatible string */ Note that struct mmc_platdata is defined in the C file, not in a header. This is to avoid needing to include dt-structs.h in a header file. The idea is to -- 2.20.1
[PATCH v4 02/14] dtoc: add missing code comments
Add missing information about internal class members in order to make the code easier to follow. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index ecfe0624d1..bc0de426a9 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -140,6 +140,9 @@ class DtbPlatdata(object): _include_disabled: true to include nodes marked status = "disabled" _outfile: The current output file (sys.stdout or a real file) _lines: Stashed list of output lines for outputting in the future +_aliases: Dict that hold aliases for compatible strings +key: First compatible string declared in a node +value: List of additional compatible strings declared in a node """ def __init__(self, dtb_fname, include_disabled): self._fdt = None -- 2.20.1
[PATCH v4 01/14] drivers: rename drivers to match compatible string
When using OF_PLATDATA, the bind process between devices and drivers is performed trying to match compatible string with driver names. However driver names are not strictly defined, and also there are different names used when declaring a driver with U_BOOT_DRIVER, the name of the symbol used in the linker list and the used in the struct driver_info. In order to make things a bit more clear, rename the drivers names. This will also help for further OF_PLATDATA improvements, such as checking for valid driver names. Signed-off-by: Walter Lozano --- .../mach-at91/arm926ejs/at91sam9260_devices.c | 6 +-- .../arm926ejs/at91sam9m10g45_devices.c| 10 ++-- arch/arm/mach-rockchip/rk3328/syscon_rk3328.c | 4 +- board/davinci/da8xxevm/omapl138_lcdk.c| 2 +- board/sandbox/sandbox.c | 2 +- drivers/clk/at91/clk-master.c | 4 +- drivers/clk/at91/clk-peripheral.c | 4 +- drivers/clk/at91/pmc.c| 4 +- drivers/core/simple-bus.c | 4 +- drivers/gpio/at91_gpio.c | 4 +- drivers/gpio/da8xx_gpio.c | 4 +- drivers/gpio/mxs_gpio.c | 6 +-- drivers/gpio/rk_gpio.c| 4 +- drivers/gpio/sandbox.c| 4 +- drivers/i2c/rk_i2c.c | 4 +- drivers/input/cros_ec_keyb.c | 4 +- drivers/misc/cros_ec_sandbox.c| 4 +- drivers/mmc/davinci_mmc.c | 4 +- drivers/mmc/mxsmmc.c | 6 +-- drivers/mmc/rockchip_dw_mmc.c | 2 +- drivers/mtd/spi/sf-uclass.c | 2 +- drivers/mtd/spi/sf_probe.c| 4 +- drivers/pinctrl/nxp/pinctrl-mxs.c | 4 +- drivers/pinctrl/pinctrl-at91.c| 4 +- drivers/pinctrl/rockchip/pinctrl-rk3188.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3288.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3328.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rk3368.c | 2 +- drivers/power/pmic/rk8xx.c| 4 +- drivers/power/regulator/fixed.c | 4 +- drivers/ram/rockchip/dmc-rk3368.c | 2 +- drivers/ram/rockchip/sdram_rk3188.c | 2 +- drivers/ram/rockchip/sdram_rk3288.c | 2 +- drivers/ram/rockchip/sdram_rk3328.c | 2 +- drivers/serial/sandbox.c | 6 +-- drivers/spi/mxs_spi.c | 6 +-- drivers/spi/rk_spi.c | 6 +-- drivers/spi/sandbox_spi.c | 4 +- drivers/tpm/tpm_tis_sandbox.c | 4 +- drivers/video/rockchip/rk3288_vop.c | 4 +- drivers/video/sandbox_sdl.c | 4 +- drivers/watchdog/at91sam9_wdt.c | 4 +- test/dm/gpio.c| 2 +- test/dm/spi.c | 6 +-- test/py/tests/test_bind.py| 54 +-- 45 files changed, 104 insertions(+), 120 deletions(-) diff --git a/arch/arm/mach-at91/arm926ejs/at91sam9260_devices.c b/arch/arm/mach-at91/arm926ejs/at91sam9260_devices.c index c033ed6d16..8122d2f98e 100644 --- a/arch/arm/mach-at91/arm926ejs/at91sam9260_devices.c +++ b/arch/arm/mach-at91/arm926ejs/at91sam9260_devices.c @@ -220,7 +220,7 @@ static const struct at91_port_platdata at91sam9260_plat[] = { }; U_BOOT_DEVICES(at91sam9260_gpios) = { - { "gpio_at91", _plat[0] }, - { "gpio_at91", _plat[1] }, - { "gpio_at91", _plat[2] }, + { "atmel_at91rm9200_gpio", _plat[0] }, + { "atmel_at91rm9200_gpio", _plat[1] }, + { "atmel_at91rm9200_gpio", _plat[2] }, }; diff --git a/arch/arm/mach-at91/arm926ejs/at91sam9m10g45_devices.c b/arch/arm/mach-at91/arm926ejs/at91sam9m10g45_devices.c index 89cbeafa20..08ca3edd78 100644 --- a/arch/arm/mach-at91/arm926ejs/at91sam9m10g45_devices.c +++ b/arch/arm/mach-at91/arm926ejs/at91sam9m10g45_devices.c @@ -176,9 +176,9 @@ static const struct at91_port_platdata at91sam9260_plat[] = { }; U_BOOT_DEVICES(at91sam9260_gpios) = { - { "gpio_at91", _plat[0] }, - { "gpio_at91", _plat[1] }, - { "gpio_at91", _plat[2] }, - { "gpio_at91", _plat[3] }, - { "gpio_at91", _plat[4] }, + { "atmel_at91rm9200_gpio", _plat[0] }, + { "atmel_at91rm9200_gpio", _plat[1] }, + { "atmel_at91rm9200_gpio", _plat[2] }, + { "atmel_at91rm9200_gpio", _plat[3] }, + { "atmel_at91rm9200_gpio", _plat[4] }, }; diff --git a/arch/arm/mach-rockchip/rk3328/syscon_rk3328.c b/arch/arm/mach-rockchip/rk3328/syscon_rk3328.c index 8a0eceb178..daf74a0e2d 100644 --- a/arch/arm/mach-rockchip/rk3328/syscon_rk3328.c +++ b/arch/arm/mach-rockchip/rk3328/syscon_rk33
[PATCH v3 14/14] dtoc: add test for cd-gpios
Add a test for dtoc taking into account the cd-gpios property. Signed-off-by: Walter Lozano --- tools/dtoc/dtoc_test_phandle_cd_gpios.dts | 42 ++ tools/dtoc/test_dtoc.py | 67 +++ 2 files changed, 109 insertions(+) create mode 100644 tools/dtoc/dtoc_test_phandle_cd_gpios.dts diff --git a/tools/dtoc/dtoc_test_phandle_cd_gpios.dts b/tools/dtoc/dtoc_test_phandle_cd_gpios.dts new file mode 100644 index 00..241743e73e --- /dev/null +++ b/tools/dtoc/dtoc_test_phandle_cd_gpios.dts @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2020 Collabora Ltd. + */ + +/dts-v1/; + +/ { + phandle: phandle-target { + u-boot,dm-pre-reloc; + compatible = "target"; + intval = <0>; + #gpio-cells = <0>; + }; + + phandle_1: phandle2-target { + u-boot,dm-pre-reloc; + compatible = "target"; + intval = <1>; + #gpio-cells = <1>; + }; + phandle_2: phandle3-target { + u-boot,dm-pre-reloc; + compatible = "target"; + intval = <2>; + #gpio-cells = <2>; + }; + + phandle-source { + u-boot,dm-pre-reloc; + compatible = "source"; + cd-gpios = < _1 11 _2 12 13 >; + }; + + phandle-source2 { + u-boot,dm-pre-reloc; + compatible = "source"; + cd-gpios = <>; + }; +}; diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 67ca9a8da1..3c8e343b1f 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -466,6 +466,73 @@ U_BOOT_DEVICE(phandle_source2) = { void dm_populate_phandle_data(void) { \tdtv_phandle_source2.clocks[0].node = DM_GET_DEVICE(phandle_target); } +''', data) + +def test_phandle_cd_gpio(self): +"""Test that phandle targets are generated when unsing cd-gpios""" +dtb_file = get_dtb_file('dtoc_test_phandle_cd_gpios.dts') +output = tools.GetOutputFilename('output') +dtb_platdata.run_steps(['platdata'], dtb_file, False, output, True) +with open(output) as infile: +data = infile.read() +self._CheckStrings(C_HEADER + ''' +static struct dtd_target dtv_phandle_target = { +\t.intval\t\t\t= 0x0, +}; +U_BOOT_DEVICE(phandle_target) = { +\t.name\t\t= "target", +\t.platdata\t= _phandle_target, +\t.platdata_size\t= sizeof(dtv_phandle_target), +}; + +static struct dtd_target dtv_phandle2_target = { +\t.intval\t\t\t= 0x1, +}; +U_BOOT_DEVICE(phandle2_target) = { +\t.name\t\t= "target", +\t.platdata\t= _phandle2_target, +\t.platdata_size\t= sizeof(dtv_phandle2_target), +}; + +static struct dtd_target dtv_phandle3_target = { +\t.intval\t\t\t= 0x2, +}; +U_BOOT_DEVICE(phandle3_target) = { +\t.name\t\t= "target", +\t.platdata\t= _phandle3_target, +\t.platdata_size\t= sizeof(dtv_phandle3_target), +}; + +static struct dtd_source dtv_phandle_source = { +\t.cd_gpios\t\t= { +\t\t\t{NULL, {}}, +\t\t\t{NULL, {11}}, +\t\t\t{NULL, {12, 13}}, +\t\t\t{NULL, {}},}, +}; +U_BOOT_DEVICE(phandle_source) = { +\t.name\t\t= "source", +\t.platdata\t= _phandle_source, +\t.platdata_size\t= sizeof(dtv_phandle_source), +}; + +static struct dtd_source dtv_phandle_source2 = { +\t.cd_gpios\t\t= { +\t\t\t{NULL, {}},}, +}; +U_BOOT_DEVICE(phandle_source2) = { +\t.name\t\t= "source", +\t.platdata\t= _phandle_source2, +\t.platdata_size\t= sizeof(dtv_phandle_source2), +}; + +void dm_populate_phandle_data(void) { +\tdtv_phandle_source.cd_gpios[0].node = DM_GET_DEVICE(phandle_target); +\tdtv_phandle_source.cd_gpios[1].node = DM_GET_DEVICE(phandle2_target); +\tdtv_phandle_source.cd_gpios[2].node = DM_GET_DEVICE(phandle3_target); +\tdtv_phandle_source.cd_gpios[3].node = DM_GET_DEVICE(phandle_target); +\tdtv_phandle_source2.cd_gpios[0].node = DM_GET_DEVICE(phandle_target); +} ''', data) def test_phandle_bad(self): -- 2.20.1
[PATCH v3 13/14] dtoc: update dtb_platdata to support cd-gpios
Currently dtoc does not support the property cd-gpios used to declare the gpios for card detect in mmc. This patch adds support to cd-gpios property. Signed-off-by: Walter Lozano --- tools/dtoc/dtb_platdata.py | 16 ++-- tools/dtoc/test_dtoc.py| 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index b25e1c17e0..6015ae734f 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -258,7 +258,7 @@ class DtbPlatdata(object): Return: Number of argument cells is this is a phandle, else None """ -if prop.name in ['clocks']: +if prop.name in ['clocks', 'cd-gpios']: if not isinstance(prop.value, list): prop.value = [prop.value] val = prop.value @@ -278,11 +278,14 @@ class DtbPlatdata(object): if not target: raise ValueError("Cannot parse '%s' in node '%s'" % (prop.name, node_name)) -prop_name = '#clock-cells' -cells = target.props.get(prop_name) +cells = None +for prop_name in ['#clock-cells', '#gpio-cells']: +cells = target.props.get(prop_name) +if cells: +break if not cells: -raise ValueError("Node '%s' has no '%s' property" % -(target.name, prop_name)) +raise ValueError("Node '%s' has no cells property" % +(target.name)) num_args = fdt_util.fdt32_to_cpu(cells.value) max_args = max(max_args, num_args) args.append(num_args) @@ -652,7 +655,8 @@ class DtbPlatdata(object): # dtv_dmc_at_xxx.clocks[0].node = DM_GET_DEVICE(clock_controller_at_xxx) self.buf('void dm_populate_phandle_data(void) {\n') for l in self._links: -self.buf('\t%s = DM_GET_DEVICE(%s);\n' % (l['var_node'], l['dev_name'])) +self.buf('\t%s = DM_GET_DEVICE(%s);\n' % + (l['var_node'], l['dev_name'])) self.buf('}\n') self.out(''.join(self.get_buf())) diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 209542c849..67ca9a8da1 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -485,7 +485,7 @@ void dm_populate_phandle_data(void) { output = tools.GetOutputFilename('output') with self.assertRaises(ValueError) as e: self.run_test(['struct'], dtb_file, output) -self.assertIn("Node 'phandle-target' has no '#clock-cells' property", +self.assertIn("Node 'phandle-target' has no cells property", str(e.exception)) def test_aliases(self): -- 2.20.1
[PATCH v3 08/14] core: extend struct driver_info to point to device
Currently when creating an U_BOOT_DEVICE entry a struct driver_info is declared, which contains the data needed to instantiate the device. However, the actual device is created at runtime and there is no proper way to get the device based on its struct driver_info. This patch extends struct driver_info adding a pointer to udevice which is populated during the bind process, allowing to generate a set of functions to get the device based on its struct driver_info. Signed-off-by: Walter Lozano --- drivers/core/device.c | 26 +++--- drivers/core/root.c | 4 include/dm/device.h | 15 +++ include/dm/platdata.h | 14 ++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/drivers/core/device.c b/drivers/core/device.c index a0ad080aaf..4f8c97a195 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -250,6 +250,7 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, { struct driver *drv; uint platdata_size = 0; + int ret; drv = lists_driver_lookup_name(info->name); if (!drv) @@ -260,9 +261,16 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, #if CONFIG_IS_ENABLED(OF_PLATDATA) platdata_size = info->platdata_size; #endif - return device_bind_common(parent, drv, info->name, - (void *)info->platdata, 0, ofnode_null(), platdata_size, - devp); + ret = device_bind_common(parent, drv, info->name, +(void *)info->platdata, 0, ofnode_null(), +platdata_size, devp); + if (ret) + return ret; +#if CONFIG_IS_ENABLED(OF_PLATDATA) + info->dev = *devp; +#endif + + return ret; } static void *alloc_priv(int size, uint flags) @@ -727,6 +735,18 @@ int device_get_global_by_ofnode(ofnode ofnode, struct udevice **devp) return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp); } +#if CONFIG_IS_ENABLED(OF_PLATDATA) +int device_get_by_driver_info(const struct driver_info *info, + struct udevice **devp) +{ + struct udevice *dev; + + dev = info->dev; + + return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp); +} +#endif + int device_find_first_child(const struct udevice *parent, struct udevice **devp) { if (list_empty(>child_head)) { diff --git a/drivers/core/root.c b/drivers/core/root.c index c9ee56478a..2643ef68a7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -346,6 +346,10 @@ int dm_init_and_scan(bool pre_reloc_only) { int ret; +#if CONFIG_IS_ENABLED(OF_PLATDATA) + dm_populate_phandle_data(); +#endif + ret = dm_init(IS_ENABLED(CONFIG_OF_LIVE)); if (ret) { debug("dm_init() failed: %d\n", ret); diff --git a/include/dm/device.h b/include/dm/device.h index 2cfe10766f..f5738a0cee 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -538,6 +538,21 @@ int device_find_global_by_ofnode(ofnode node, struct udevice **devp); */ int device_get_global_by_ofnode(ofnode node, struct udevice **devp); +/** + * device_get_by_driver_info() - Get a device based on driver_info + * + * Locates a device by its struct driver_info, by using its reference which + * is updated during the bind process. + * + * The device is probed to activate it ready for use. + * + * @info: Struct driver_info + * @devp: Returns pointer to device if found, otherwise this is set to NULL + * @return 0 if OK, -ve on error + */ +int device_get_by_driver_info(const struct driver_info *info, + struct udevice **devp); + /** * device_find_first_child() - Find the first child of a device * diff --git a/include/dm/platdata.h b/include/dm/platdata.h index c972fa6936..cab93b071b 100644 --- a/include/dm/platdata.h +++ b/include/dm/platdata.h @@ -22,12 +22,14 @@ * @name: Driver name * @platdata: Driver-specific platform data * @platdata_size: Size of platform data structure + * @dev: Device created from this structure data */ struct driver_info { const char *name; const void *platdata; #if CONFIG_IS_ENABLED(OF_PLATDATA) uint platdata_size; + struct udevice *dev; #endif }; @@ -43,4 +45,16 @@ struct driver_info { #define U_BOOT_DEVICES(__name) \ ll_entry_declare_list(struct driver_info, __name, driver_info) +/* Get a pointer to a given driver */ +#define DM_GET_DEVICE(__name) \ + ll_entry_get(struct driver_info, __name, driver_info) + +/** + * dm_populate_phandle_data() - Populates phandle data in platda + * + * This populates phandle data with an U_BOOT_DEVICE entry get by + * DM_GET_DEVICE. The implementation of this function will be done + * by dtoc when parsing dtb. + */ +void dm_populate_phandle_data(void); #endif -- 2.20.1
[PATCH v3 12/14] arm: dts: include gpio nodes for card detect
Several MMC drivers use GPIO for card detection with cd-gpios property in the MMC node pointing to a GPIO node. However, as U-Boot tries to save space by keeping only required nodes using u-boot* properties, several devices tree result in having only in the MMC node but not the GPIO node associated to cd-gpios. This patch, fixes several ocurrence of this issue. Signed-off-by: Walter Lozano --- arch/arm/dts/da850-evm-u-boot.dtsi| 4 arch/arm/dts/da850-lcdk-u-boot.dtsi | 4 arch/arm/dts/rk3288-u-boot.dtsi | 4 arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi | 2 +- arch/arm/dts/rk3288-veyron-u-boot.dtsi| 11 +++ 5 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 arch/arm/dts/rk3288-veyron-u-boot.dtsi diff --git a/arch/arm/dts/da850-evm-u-boot.dtsi b/arch/arm/dts/da850-evm-u-boot.dtsi index d9afc5edf4..d588628641 100644 --- a/arch/arm/dts/da850-evm-u-boot.dtsi +++ b/arch/arm/dts/da850-evm-u-boot.dtsi @@ -39,3 +39,7 @@ { u-boot,dm-spl; }; + + { + u-boot,dm-spl; +}; diff --git a/arch/arm/dts/da850-lcdk-u-boot.dtsi b/arch/arm/dts/da850-lcdk-u-boot.dtsi index b372d06ca9..d50775c173 100644 --- a/arch/arm/dts/da850-lcdk-u-boot.dtsi +++ b/arch/arm/dts/da850-lcdk-u-boot.dtsi @@ -28,3 +28,7 @@ { u-boot,dm-spl; }; + + { + u-boot,dm-spl; +}; diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi index 6d31735362..51b6e018bd 100644 --- a/arch/arm/dts/rk3288-u-boot.dtsi +++ b/arch/arm/dts/rk3288-u-boot.dtsi @@ -43,3 +43,7 @@ { u-boot,dm-pre-reloc; }; + + { + u-boot,dm-pre-reloc; +}; diff --git a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi index eccc069368..251fbdee71 100644 --- a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi +++ b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi @@ -3,7 +3,7 @@ * Copyright 2015 Google, Inc */ -#include "rk3288-u-boot.dtsi" +#include "rk3288-veyron-u-boot.dtsi" { rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d diff --git a/arch/arm/dts/rk3288-veyron-u-boot.dtsi b/arch/arm/dts/rk3288-veyron-u-boot.dtsi new file mode 100644 index 00..899fe6e7a0 --- /dev/null +++ b/arch/arm/dts/rk3288-veyron-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright 2015 Google, Inc + */ + +#include "rk3288-u-boot.dtsi" + + { + u-boot,dm-pre-reloc; +}; + -- 2.20.1
[PATCH v3 10/14] dtoc: extend dtoc to use struct driver_info when linking nodes
In the current implementation, when dtoc parses a dtb to generate a struct platdata it converts the information related to linked nodes as pointers to struct platdata of destination nodes. By doing this, it makes difficult to get pointer to udevices created based on these information. This patch extends dtoc to use struct driver_info when populating information about linked nodes, which makes it easier to later get the devices created. In this context, reimplement functions like clk_get_by_index_platdata() which made use of the previous approach. Signed-off-by: Walter Lozano --- drivers/clk/clk-uclass.c| 11 ++- drivers/misc/irq-uclass.c | 10 ++- drivers/mmc/ftsdc010_mci.c | 2 +- drivers/mmc/rockchip_dw_mmc.c | 2 +- drivers/mmc/rockchip_sdhci.c| 2 +- drivers/ram/rockchip/sdram_rk3399.c | 2 +- drivers/spi/rk_spi.c| 2 +- include/clk.h | 4 +- tools/dtoc/dtb_platdata.py | 26 ++- tools/dtoc/test_dtoc.py | 104 10 files changed, 100 insertions(+), 65 deletions(-) diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 71878474eb..412f26cd29 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -25,17 +25,16 @@ static inline const struct clk_ops *clk_dev_ops(struct udevice *dev) #if CONFIG_IS_ENABLED(OF_CONTROL) # if CONFIG_IS_ENABLED(OF_PLATDATA) -int clk_get_by_index_platdata(struct udevice *dev, int index, - struct phandle_1_arg *cells, struct clk *clk) +int clk_get_by_driver_info(struct udevice *dev, struct phandle_1_arg *cells, + struct clk *clk) { int ret; - if (index != 0) - return -ENOSYS; - ret = uclass_get_device(UCLASS_CLK, 0, >dev); + ret = device_get_by_driver_info((struct driver_info *)cells->node, + >dev); if (ret) return ret; - clk->id = cells[0].arg[0]; + clk->id = cells->arg[0]; return 0; } diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c index 61aa10e465..3c38681108 100644 --- a/drivers/misc/irq-uclass.c +++ b/drivers/misc/irq-uclass.c @@ -63,17 +63,15 @@ int irq_read_and_clear(struct irq *irq) } #if CONFIG_IS_ENABLED(OF_PLATDATA) -int irq_get_by_index_platdata(struct udevice *dev, int index, - struct phandle_1_arg *cells, struct irq *irq) +int irq_get_by_driver_info(struct udevice *dev, + struct phandle_1_arg *cells, struct irq *irq) { int ret; - if (index != 0) - return -ENOSYS; - ret = uclass_get_device(UCLASS_IRQ, 0, >dev); + ret = device_get_by_driver_info(cells->node, >dev); if (ret) return ret; - irq->id = cells[0].arg[0]; + irq->id = cells->arg[0]; return 0; } diff --git a/drivers/mmc/ftsdc010_mci.c b/drivers/mmc/ftsdc010_mci.c index 9c15eb36d6..efa92d48be 100644 --- a/drivers/mmc/ftsdc010_mci.c +++ b/drivers/mmc/ftsdc010_mci.c @@ -437,7 +437,7 @@ static int ftsdc010_mmc_probe(struct udevice *dev) chip->priv = dev; chip->dev_index = 1; memcpy(priv->minmax, dtplat->clock_freq_min_max, sizeof(priv->minmax)); - ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, >clk); + ret = clk_get_by_driver_info(dev, dtplat->clocks, >clk); if (ret < 0) return ret; #endif diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index ac710324c8..80432ddbbc 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -120,7 +120,7 @@ static int rockchip_dwmmc_probe(struct udevice *dev) priv->minmax[0] = 40; /* 400 kHz */ priv->minmax[1] = dtplat->max_frequency; - ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, >clk); + ret = clk_get_by_driver_info(dev, dtplat->clocks, >clk); if (ret < 0) return ret; #else diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index b440996b26..b073f1a08d 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -46,7 +46,7 @@ static int arasan_sdhci_probe(struct udevice *dev) host->name = dev->name; host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]); max_frequency = dtplat->max_frequency; - ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, ); + ret = clk_get_by_driver_info(dev, dtplat->clocks, ); #else max_frequency = dev_read_u32_default(dev, "max-frequency", 0); ret = clk_get_by_index(dev, 0, ); diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index d69ef01d08..87ec25f893 100644 --- a/drivers/r
[PATCH v3 11/14] dm: doc: update of-plat with new phandle support
Update documentation to reflect the new phandle support when OF_PLATDATA is used. Now phandles are implemented as pointers to U_BOOT_DEVICE, which makes it possible to get a pointer to the actual device. Signed-off-by: Walter Lozano --- doc/driver-model/of-plat.rst | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/doc/driver-model/of-plat.rst b/doc/driver-model/of-plat.rst index 376d4409a5..1e3fad137b 100644 --- a/doc/driver-model/of-plat.rst +++ b/doc/driver-model/of-plat.rst @@ -69,9 +69,8 @@ strictly necessary. Notable problems include: - Correct relations between nodes are not implemented. This means that parent/child relations (like bus device iteration) do not work yet. Some phandles (those that are recognised as such) are converted into - a pointer to platform data. This pointer can potentially be used to - access the referenced device (by searching for the pointer value). - This feature is not yet implemented, however. + a pointer to struct driver_info. This pointer can be used to access + the referenced device. How it works @@ -146,10 +145,10 @@ and the following device declaration: .clock_freq_min_max = {0x61a80, 0x8f0d180}, .vmmc_supply= 0xb, .num_slots = 0x1, -.clocks = {{_clock_controller_at_ff76, 456}, - {_clock_controller_at_ff76, 68}, - {_clock_controller_at_ff76, 114}, - {_clock_controller_at_ff76, 118}}, +.clocks = {{NULL, 456}, + {NULL, 68}, + {NULL, 114}, + {NULL, 118}}, .cap_mmc_highspeed = true, .disable_wp = true, .bus_width = 0x4, @@ -164,6 +163,13 @@ and the following device declaration: .platdata_size = sizeof(dtv_dwmmc_at_ff0c), }; +void dm_populate_phandle_data(void) { +dtv_dwmmc_at_ff0c.clocks[0].node = DM_GET_DEVICE(clock_controller_at_ff76); +dtv_dwmmc_at_ff0c.clocks[1].node = DM_GET_DEVICE(clock_controller_at_ff76); +dtv_dwmmc_at_ff0c.clocks[2].node = DM_GET_DEVICE(clock_controller_at_ff76); +dtv_dwmmc_at_ff0c.clocks[3].node = DM_GET_DEVICE(clock_controller_at_ff76); +} + The device is then instantiated at run-time and the platform data can be accessed using: @@ -329,7 +335,9 @@ prevents them being used inadvertently. All usage must be bracketed with #if CONFIG_IS_ENABLED(OF_PLATDATA). The dt-platdata.c file contains the device declarations and is is built in -spl/dt-platdata.c. +spl/dt-platdata.c. It additionally contains the definition of +dm_populate_phandle_data() which is responsible of filling the phandle +information by adding references to U_BOOT_DEVICE by using DM_GET_DEVICE The beginnings of a libfdt Python module are provided. So far this only implements a subset of the features. -- 2.20.1