Re: [PATCH v2 3/3] staging: nvec: add device tree support

[Julian Andres Klode <jak-4HMq4SXA452hPH1hqNUYSQ@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2302 bytes --]

On Thu, Oct 27, 2011 at 12:17:25PM -0700, Stephen Warren wrote:
> Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:
> > This adds device tree support to the nvec driver. By using this method
> > it is no longer necessary to specify platform data through a board
> > file.
> 
> You should document the binding in Documentation/devicetree/bindings.
> 
> > @@ -892,6 +915,17 @@ static int tegra_nvec_resume(struct platform_device *pdev)
> >  #define tegra_nvec_resume NULL
> >  #endif
> > 
> > +#if defined(CONFIG_OF)
> 
> I think you can just remove the ifdef and always include this code. Yes, it'll
> result in slightly more rodata when !CONFIG_OF, but !CONFIG_OF isn't going to
> exist or be useful for Tegra for that much longer.

Often things do not actually build anymore without CONFIG_OF -- For example,
the code in -next failed to build a month ago or so (don't know if that's
still the case, though).
 
> > +/* Match table for of_platform binding */
> > +static const struct of_device_id nvidia_nvec_of_match[] __devinitconst = {
> > +	{ .compatible = "nvidia,nvec", },
> 
> I'm not sure that nvidia,nvec is the right value, but need a little more
> background.
> 
> It's my understanding that how this works is a little micro-controller
> exists on the board, handles various devices like the keyboard, and sends
> data to Tegra by making I2C master transactions. Isn't it the case that
> the micro-controller (or at least the SW running on it) is board-specific,
> and the same for the I2C protocol? If so, nvidia,nvec is a little generic;
> we probably need to name it compal,paz00-ec or something like that?

nvec means Nvidia Embedded Controller, and the protocol is not
device-specific anyway. There are some device-specific things,
the controller has some commands reserved for OEM usage. We do
not use those yet.

For an example of a non-paz00 device: The Advent Vega tablet and
similar (those with boards called "shuttle") also use nvec.

If you want to know details, you could search someone at your
company who knows more about it. There should be a few people
knowing things about nvec.

-- 
Julian Andres Klode  - Debian Developer, Ubuntu Member

See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]