From mboxrd@z Thu Jan 1 00:00:00 1970 From: Walter Lozano Date: Sun, 26 Jul 2020 23:16:12 -0300 Subject: [RFC 3/4] dtoc: add support for generate stuct udevice_id In-Reply-To: References: <20200619211140.5081-1-walter.lozano@collabora.com> <20200619211140.5081-4-walter.lozano@collabora.com> <7a17bbb9-a537-09a8-8d18-3f7438ad818d@collabora.com> Message-ID: <7eab15a5-ebac-f9f6-184e-a77d47578837@collabora.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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-agust at 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-agust at 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", &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