Re: [PATCH] mfd: cros ec: spi: Add delay for raising CS

[Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]

> > > From: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > 
> > > The EC has specific timing it requires. Add support for an optional delay
> > > after raising CS to fix timing issues. This is configurable based on
> > > a DT property "google,cros-ec-spi-msg-delay".
> > > 
> > > If this property isn't set, then no delay will be added. However, if set
> > > it will cause a delay equal to the value passed to it to be inserted at
> > > the end of a transaction.
> > > 
> > > Signed-off-by: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > Reviewed-by: Bernie Thompson <bhthompson-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > Reviewed-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > > Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> > > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > > Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> > > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > ---
> > > Changes in v2:
> > > - make property description more verbose
> > > 
> > >  Documentation/devicetree/bindings/mfd/cros-ec.txt |  9 +++++++
> > 
> > We need a DT dude to look over this.
> 
> I think Mark Rutland looked at this last week and I think I've addressed
> all his comments. Hopefully he'll find the time to review this.

Right, I just need his (or one of the other guy's) Ack(s) before I can
apply the patch.

> > > +	/* 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? Also, how
often is this used without DT?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html