Hi Walter,
On Sun, 26 Jul 2020 at 20:16, Walter Lozano walter.lozano@collabora.com wrote:
Hi Simon,
On 26/7/20 11:53, Simon Glass wrote:
Hi Walter,
On Tue, 7 Jul 2020 at 08:08, Walter Lozano walter.lozano@collabora.com wrote:
Hi Simon
On 6/7/20 16:21, Simon Glass wrote:
Hi Walter,
On Fri, 19 Jun 2020 at 15:12, Walter Lozano walter.lozano@collabora.com 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@...
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@...
Signed-off-by: Walter Lozano walter.lozano@collabora.com
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.
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.
I think we can put that data in the dt-platdata.c file perhaps.
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.
Regards, Simon