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

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

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

On Wed, Aug 12, 2020 at 02:40:05PM +0200, Ahmad Fatoum wrote:
> Hello,
> 
> On 8/12/20 10:55 AM, Oleksij Rempel wrote:
> > A usual board file contains at least one of_machine_is_compatible().
> > Some of the have a rather long list with complicated version logic.
> > 
> > To avoid own implementation for driver management, register the root node
> > of device tree as platform device. So, the main platform bus can attach
> > proper board driver. After this patch a typical board.c file can reuse
> > existing driver infrastructure.
> > 
> > After this patch, you will be able to see all registered board drivers
> > with drvinfo as fallow:
> > ...
> > board-embest-riot
> > board-protonic-imx6
> >     dt-root.of
> > ...
> > 
> > With devinfo, you'll be able to get some board specific information,
> > if this is implemented:
> > barebox@Protonic PRTI6Q board:/ devinfo dt-root.of
> > Driver: board-protonic-imx6
> > Bus: platform
> > Parameters:
> >   boardid: 0 (type: uint32)
> >   boardrev: 1 (type: uint32)
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  drivers/of/base.c     | 2 ++
> >  drivers/of/platform.c | 5 +++++
> >  2 files changed, 7 insertions(+)
> > 
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 4c633bcd49..6f6cdc9379 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -2149,6 +2149,8 @@ int of_probe(void)
> >  	if (firmware)
> >  		of_platform_populate(firmware, NULL, NULL);
> >  
> > +	of_platform_device_create(root_node, NULL);
> > +
> >  	of_clk_init(root_node, NULL);
> >  	of_platform_populate(root_node, of_default_bus_match_table, NULL);
> >  
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index ca84cede23..767beebe06 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -50,6 +50,11 @@ static void of_device_make_bus_id(struct device_d *dev)
> >  	const __be32 *reg;
> >  	u64 addr;
> >  
> > +	if (!node->parent) {
> > +		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.

> > +		return;
> > +	}
> 
> 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.

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