Re: [PATCH fixup v1] of: base: register DT root as device

[Oleksij Rempel <o.rempel@pengutronix.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2865 bytes --]

On Wed, Aug 12, 2020 at 05:37:33PM +0200, Ahmad Fatoum wrote:
> Hello,
> 
> On 8/12/20 5:13 PM, Oleksij Rempel wrote:
> >>> +		dev_set_name(dev, "dt-root.of");
> >>
> >> Couldn't we drop the dt-? just let it be root.of.
> >> dashes make use of device parameters less convenient should we
> >> want to use those in future IIRC.
> > 
> > dt is used to make clear: it is root of dt and not some random root of
> > what ever.
> 
> It's redundant, there is already a .of suffix.
> I like machine.of more though.
> 
> >> of_platform_device_create does:
> >>
> >> [-] check if device is available: not applicable to root node
> >> [-] populate io resources: not applicable to root node
> >> [-] use of_device_make_bus_id to get a name: not applicable to root node (prior to this patch)
> >> [-] configure dma: not applicable to root node
> >> [x] call platform_device_register
> > 
> > You make this assumption, just because this node has no parents?
> > Does it means, a parent less child may have no resources to do some work?
> > You should be ashamed of yourself! :D
> > 
> > But really, what prevents you to assign board specific resource to a
> > root node. It is just node as many others.
> 
> It makes no sense for the root node to have resources.
> What is a machine-wide interrupt? Or a machine-wide MMIO region?
> What size would that region even have, when you have no parent
> bus that defines address/size cells?

yes, you are right.

> Do you have any examples of oftree resources for the root node?

Do you have any example of the root node used as device?

> I'd rather not litter core code with an if-clause that evaluates to
> true only once,

How many ifs are added in this patch and how many ifs will by added by
your suggestion? 

> to support your (IMHO wrong)  use of a helper.

Interesting change of conversation. Please stay technical.

> of_device_make_bus_id is taken from Linux and does per comment:
> 
> This routine will first try using the translated bus address to
> derive a unique name. If it cannot, then it will prepend names from
> parent nodes until a unique name can be derived.
> 
> IMO, it should stay that way.

Ok, i'll send a patch to rename of_device_make_bus_id to of_device_make_id.
In this case it will reflect new reality and keep the code readable.

If you have arguments in following topics:
- it will significantly affect performance
- it will affect size of executable
- it will affect maintainability

Please use them

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox