On Mon, Nov 25, 2013 at 12:10:35PM +0000, Lee Jones wrote:
> > > > > > + /* Check for any DT properties */
> > > > > > + if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> > > > >
> > > > > No need for the first check.
> > > >
> > > > Why not? While it is true that dev->of_node would be enough to determine
> > > > that the device was instantiated from a device tree, the IS_ENABLED()
> > > > will allow the compiler to throw away cros_ec_spi_dt_probe() if OF isn't
> > > > enabled. At the same time it's nicer than #ifdeffery sprinkled across
> > > > the file and it actually compile-tests all the code. Win-win-win, isn't
> > > > it?
> > >
> > > I agree that it's better than #ifdeffery, but I didn't know that if
> > > this check tested negative that the subordinate method would be
> > > optimised out by the compiler. Are you sure that happens?
> >
> > I'm pretty sure that's what happens. If you have code like this (which
> > is pretty much what the above evaluates to if !OF):
> >
> > static void foo(void)
> > {
> > ...
> > }
> >
> > void bar(void)
> > {
> > if (0)
> > foo();
> > }
> >
> > Then the compiler would be actively stupid if it kept foo around.
>
> +1
>
> > > Also, how often is this used without DT?
> >
> > Well, it doesn't have a dependency on OF, so it can be used without it.
> > In-tree there seems to be no indication that it is used without DT, but
> > given that there's no dependency it makes sense to keep it optional.
> >
> > Of course we could also add the dependency if it really isn't used
> > outside of a DT context.
>
> This would obviously be my preference. I'd be surprised if anyone was
> using this with platform data, but as you say, you never know. Perhaps
> it would be worth obtaining for clarification from the Google guys?
Bernie, Andrew, Rhyland, can anyone of you comment. Are you aware of
this driver being used with a non-DT setup? I suspect maybe on Intel
Chromebooks it might be, but I couldn't find any evidence of that in
the upstream kernel.
In fact the only upstream user of cros-ec seems to be exynos5250-snow,
which I think is one of the Chromebooks, but it uses DT as well.
Are there any objections to removing non-DT support from the driver?
Thierry
Attachment:
pgpIoc2EY6p3Q.pgp
Description: PGP signature
