mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 2/6] ARM: introduce machine description
Date: Mon, 2 Dec 2013 12:54:09 +0100	[thread overview]
Message-ID: <20131202115409.GG24559@pengutronix.de> (raw)
In-Reply-To: <20131202110452.GC27628@ns203013.ovh.net>

On Mon, Dec 02, 2013 at 12:04:52PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 10:01 Mon 02 Dec     , Sascha Hauer wrote:
> > >  	struct device_node *root;
> > >  	void *fdt;
> > > @@ -48,6 +49,10 @@ static int of_arm_init(void)
> > >  	}
> > >  
> > >  	root = of_unflatten_dtb(NULL, fdt);
> > > +
> > > +	if (arm_set_dt_machine(NULL))
> > > +		pr_debug("No compatible machine found\n");
> > 
> > Why call this with NULL and not the compatible string found in the just
> > unflattened dt?
> 
> if NULL the code check for the compatible node
> 
> otherwise for a specific compatible passs as arg

As said, it can't work when you call arm_set_dt_machine() before
of_set_root_node() as you do here.

> > > +int is_dt_compatible(const struct machine_desc *m, const char* dt_compat)
> > > +{
> > > +	const char *const *dtc = m->dt_compat;
> > > +
> > > +	while (dtc) {

*dtc btw. dtc will always be true, at least until it wraps around 32bit.

> > > +		const char *s = *dtc;
> > > +		if (dt_compat && !of_compat_cmp(s, dt_compat, strlen(dt_compat)))
> > > +			return 1;
> > > +		else if (of_machine_is_compatible(s))
> > > +			return 1;
> > > +		dtc++;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int arm_set_dt_machine(const char* dt_compat)
> > 
> > I don't really understand how this function behaves. It should set the
> > machine desc based on dt_compat, but seems to fall back to some other
> > mechanism when dt_compat is NULL. Looking at it this function is only
> 
> simple 2 case
> 
> 1 check the compatible node
> 
> 1 check a specific comp if pas as arg
> 
> > called with NULL argument in your code, so it seems like it depends on
> > of_machine_is_compatible(), but this can only return something valid
> > when the root_node has been set. You call arm_set_dt_machine() before
> > of_set_root_node() so this can't work.
> > 
> > Matching a machine_desc to a devicetree is more complicated. You can't
> > return the first match, but instead have to find the best match. If you
> > have multiple i.MX6 machines, then all will match to fsl,imx6. You have
> > to find the correct board though.
> 
> no they will have their own compatible in the DT_MACHINE_DESC

Look for example in a current kernel. The SH Mobile koelsch board has:

	compatible = "renesas,koelsch", "renesas,r8a7791";

The kernel has a specific koelsch machine descriptor:

> static const char * const koelsch_boards_compat_dt[] __initconst = {
> 	"renesas,koelsch",
> 	NULL,
> };
> 
> DT_MACHINE_START(KOELSCH_DT, "koelsch")
> 	.smp		= smp_ops(r8a7791_smp_ops),
> 	.init_early	= r8a7791_init_early,
> 	.init_machine	= koelsch_add_standard_devices,
> 	.init_time	= rcar_gen2_timer_init,
> 	.dt_compat	= koelsch_boards_compat_dt,
> MACHINE_END

Additionally there is a generic SoC machine descriptor:

> static const char *r8a7791_boards_compat_dt[] __initdata = {
> 	"renesas,r8a7791",
> 	NULL,
> };
> 
> DT_MACHINE_START(R8A7791_DT, "Generic R8A7791 (Flattened Device Tree)")
> 	.smp		= smp_ops(r8a7791_smp_ops),
> 	.init_early	= r8a7791_init_early,
> 	.init_time	= rcar_gen2_timer_init,
> 	.dt_compat	= r8a7791_boards_compat_dt,
> MACHINE_END

Now of course when you have a koelsch board and koelsch support compiled
in you want to match the renesas,koelsch machine entry and not the
renesas,r8a7791 entry.

The kernel does this with of_flat_dt_match_machine().

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

  reply	other threads:[~2013-12-02 11:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-28 18:05 [PATCH 0/6] ARM: machine struct support Jean-Christophe PLAGNIOL-VILLARD
2013-11-28 18:06 ` [PATCH 1/6] arm: gen-mach-types: add MAX_MACH_TYPE Jean-Christophe PLAGNIOL-VILLARD
2013-11-28 18:06   ` [PATCH 2/6] ARM: introduce machine description Jean-Christophe PLAGNIOL-VILLARD
2013-11-28 18:19     ` Alexander Aring
2013-11-28 19:00       ` Jean-Christophe PLAGNIOL-VILLARD
2013-11-28 19:02         ` Alexander Aring
2013-12-02  9:01     ` Sascha Hauer
2013-12-02 11:04       ` Jean-Christophe PLAGNIOL-VILLARD
2013-12-02 11:54         ` Sascha Hauer [this message]
2013-11-28 18:06   ` [PATCH 3/6] ARM: introduce SoC description Jean-Christophe PLAGNIOL-VILLARD
2013-11-28 18:06   ` [PATCH 4/6] AT91: detect SoC earlier Jean-Christophe PLAGNIOL-VILLARD
2013-11-28 18:06   ` [PATCH 5/6] AT91: switch to machine description Jean-Christophe PLAGNIOL-VILLARD
2013-11-28 18:06   ` [PATCH 6/6] vexpress: " Jean-Christophe PLAGNIOL-VILLARD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131202115409.GG24559@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=plagnioj@jcrosoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox