mbox series

[RFC,0/4] drivers: footprint reduction proposal

Message ID 20200619211140.5081-1-walter.lozano@collabora.com
Headers show
Series drivers: footprint reduction proposal | expand

Message

Walter Lozano June 19, 2020, 9:11 p.m. UTC
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

Walter Lozano (4):
  dtoc: add POC for dtb shrink
  dtoc: add initial support for deleting DTB nodes
  dtoc: add support for generate stuct udevice_id
  mmc: fsl_esdhc_imx: make use of dtoc to generate struct udevice_id

 drivers/mmc/fsl_esdhc_imx.c |  58 ++++++++++++++------
 tools/dtoc/dtb_platdata.py  | 102 ++++++++++++++++++++++++++++++++++--
 tools/dtoc/fdt.py           |   3 ++
 3 files changed, 143 insertions(+), 20 deletions(-)

Comments

Tom Rini June 19, 2020, 9:48 p.m. UTC | #1
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
<generated/compatible.h> so is something missing?  Thanks!
Walter Lozano June 22, 2020, 2:12 p.m. UTC | #2
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
> <generated/compatible.h> 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.

Regards,

Walter
Tom Rini June 22, 2020, 2:20 p.m. UTC | #3
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
> > <generated/compatible.h> 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!
Walter Lozano June 22, 2020, 3:25 p.m. UTC | #4
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
>>> <generated/compatible.h> 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.

Regards,
Walter Lozano June 26, 2020, 7:17 p.m. UTC | #5
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
>>>> <generated/compatible.h> 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