| Message ID | b5f0dc6e7996a21d4493f8e7765e322fe6d8b7ef.1319313020.git.marvin24@gmx.de |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Sat, Oct 22, 2011 at 1:16 PM, Marc Dietrich <marvin24@gmx.de> wrote: > This adds support for the embedded controller known as NVEC. The driver > lives currently in the staging tree and we aim to promote it one level > higher in the near future. > > The NVEC driver uses the I2C resources of the master controller for now > until slave support is added to the i2c-tegra driver. > diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c > index 602f8dd..3f46b37 100644 > --- a/arch/arm/mach-tegra/board-paz00.c > +++ b/arch/arm/mach-tegra/board-paz00.c > @@ -44,6 +44,8 @@ > #include "devices.h" > #include "gpio-names.h" > > +#include "../../../drivers/staging/nvec/nvec.h" Ick, no! Move the header file containing platform data to include/linux/platform_data instead (or break it off in a separate header file). > static struct platform_device *paz00_devices[] __initdata = { > &debug_uart, > &tegra_sdhci_device4, > @@ -127,6 +134,10 @@ static void paz00_i2c_init(void) > platform_device_register(&tegra_i2c_device1); > platform_device_register(&tegra_i2c_device2); > platform_device_register(&tegra_i2c_device4); > + > + tegra_i2c_device3.name = "nvec"; > + tegra_i2c_device3.dev.platform_data = &nvec_pdata; > + platform_device_register(&tegra_i2c_device3); Please define a separate platform_device instead of hijacking the current one, please. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday 22 October 2011 13:27:12 Olof Johansson wrote: > On Sat, Oct 22, 2011 at 1:16 PM, Marc Dietrich <marvin24@gmx.de> wrote: > > This adds support for the embedded controller known as NVEC. The driver > > lives currently in the staging tree and we aim to promote it one level > > higher in the near future. > > > > The NVEC driver uses the I2C resources of the master controller for now > > until slave support is added to the i2c-tegra driver. > > > > diff --git a/arch/arm/mach-tegra/board-paz00.c > > b/arch/arm/mach-tegra/board-paz00.c index 602f8dd..3f46b37 100644 > > --- a/arch/arm/mach-tegra/board-paz00.c > > +++ b/arch/arm/mach-tegra/board-paz00.c > > @@ -44,6 +44,8 @@ > > #include "devices.h" > > #include "gpio-names.h" > > > > +#include "../../../drivers/staging/nvec/nvec.h" > > Ick, no! Move the header file containing platform data to > include/linux/platform_data instead (or break it off in a separate > header file). I know this looks ugly, but it is AFAIK the only (and the common) way for a staging driver to be used. Of course the header will be moved to e.g. to include/linux/mfd once the driver is ready for mainline, but till that we just cannot write somewhere outside of the staging dir. > > static struct platform_device *paz00_devices[] __initdata = { > > &debug_uart, > > &tegra_sdhci_device4, > > @@ -127,6 +134,10 @@ static void paz00_i2c_init(void) > > platform_device_register(&tegra_i2c_device1); > > platform_device_register(&tegra_i2c_device2); > > platform_device_register(&tegra_i2c_device4); > > + > > + tegra_i2c_device3.name = "nvec"; > > + tegra_i2c_device3.dev.platform_data = &nvec_pdata; > > + platform_device_register(&tegra_i2c_device3); > > Please define a separate platform_device instead of hijacking the > current one, please. ok, I just wanted to keep the patch small ;-) Marc -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[+gregkh] On Sat, Oct 22, 2011 at 10:49:42PM +0200, Marc Dietrich wrote: > On Saturday 22 October 2011 13:27:12 Olof Johansson wrote: > > On Sat, Oct 22, 2011 at 1:16 PM, Marc Dietrich <marvin24@gmx.de> wrote: > > > This adds support for the embedded controller known as NVEC. The driver > > > lives currently in the staging tree and we aim to promote it one level > > > higher in the near future. > > > > > > The NVEC driver uses the I2C resources of the master controller for now > > > until slave support is added to the i2c-tegra driver. > > > > > > diff --git a/arch/arm/mach-tegra/board-paz00.c > > > b/arch/arm/mach-tegra/board-paz00.c index 602f8dd..3f46b37 100644 > > > --- a/arch/arm/mach-tegra/board-paz00.c > > > +++ b/arch/arm/mach-tegra/board-paz00.c > > > @@ -44,6 +44,8 @@ > > > #include "devices.h" > > > #include "gpio-names.h" > > > > > > +#include "../../../drivers/staging/nvec/nvec.h" > > > > Ick, no! Move the header file containing platform data to > > include/linux/platform_data instead (or break it off in a separate > > header file). > > I know this looks ugly, but it is AFAIK the only (and the common) way for a > staging driver to be used. Of course the header will be moved to e.g. to > include/linux/mfd once the driver is ready for mainline, but till that we just > cannot write somewhere outside of the staging dir. Actually, you should be OK with adding it to include/linux/platform_data. But if you're just about to add device tree, it might make sense to do a device tree binding instead and only do device-tree configuration of the device instead. Care to cook up a patch for that? > > > + tegra_i2c_device3.name = "nvec"; > > > + tegra_i2c_device3.dev.platform_data = &nvec_pdata; > > > + platform_device_register(&tegra_i2c_device3); > > > > Please define a separate platform_device instead of hijacking the > > current one, please. > > ok, I just wanted to keep the patch small ;-) Keeping the code clean is more important than keeping the change small. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM: > This adds support for the embedded controller known as NVEC. The driver > lives currently in the staging tree and we aim to promote it one level > higher in the near future. > > The NVEC driver uses the I2C resources of the master controller for now > until slave support is added to the i2c-tegra driver. I'm in two minds about this: The fact that the nvec driver accesses the I2C HW directly is a hack; that should all happen in the I2C driver, and the nvec code should /just/ handle the protocol to talk to the microcontroller. To that end, we're starting to bring on some more engineers internally who will aim to upstream more of our downstream kernel. Part of this includes upstreaming the I2C slave support in a more palatable fashion. I know those engineers are keen in general to get started, but I can't yet estimate a timescale for when this particular change will be ready. That said, I'm not sure that I want to block progress on getting nvec and the AC100 support going. Olof, do you have any input here? Thanks.
On Tue, Oct 25, 2011 at 7:06 AM, Stephen Warren <swarren@nvidia.com> wrote: > Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM: >> This adds support for the embedded controller known as NVEC. The driver >> lives currently in the staging tree and we aim to promote it one level >> higher in the near future. >> >> The NVEC driver uses the I2C resources of the master controller for now >> until slave support is added to the i2c-tegra driver. > > I'm in two minds about this: > > The fact that the nvec driver accesses the I2C HW directly is a hack; that > should all happen in the I2C driver, and the nvec code should /just/ handle > the protocol to talk to the microcontroller. > > To that end, we're starting to bring on some more engineers internally who > will aim to upstream more of our downstream kernel. Part of this includes > upstreaming the I2C slave support in a more palatable fashion. I know those > engineers are keen in general to get started, but I can't yet estimate a > timescale for when this particular change will be ready. > > That said, I'm not sure that I want to block progress on getting nvec > and the AC100 support going. Olof, do you have any input here? Thanks. Yeah, let's not hold up ac100 based on not-yet-ready code, especially since the driver is in staging. If the i2c slave driver could be done in time to be there before nvec graduates from staging, that would probably be ideal. That being said, let's do it reasonably clean -- Marc, if you guys are up for trying to define device-tree bindings for this that would be awesome, and move to that for driver probing. That avoids adding an include/linux/platform_data header file. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 24 October 2011 22:06:46 Stephen Warren wrote: > Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM: > > This adds support for the embedded controller known as NVEC. The driver > > lives currently in the staging tree and we aim to promote it one level > > higher in the near future. > > > > The NVEC driver uses the I2C resources of the master controller for now > > until slave support is added to the i2c-tegra driver. > > I'm in two minds about this: > > The fact that the nvec driver accesses the I2C HW directly is a hack; that > should all happen in the I2C driver, and the nvec code should /just/ handle > the protocol to talk to the microcontroller. > > To that end, we're starting to bring on some more engineers internally who > will aim to upstream more of our downstream kernel. Part of this includes > upstreaming the I2C slave support in a more palatable fashion. I know those > engineers are keen in general to get started, but I can't yet estimate a > timescale for when this particular change will be ready. yeah, we just found out by chance that there exists a slave implementation at NVIDIA since last year :-( <http://nv- tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=commit;h=fe0261ca61a134fc13ae9c0b2a70fd63804a516e> but it doesn't seem to be upstream ready yet (maybe should be merged with i2c- tegra driver). -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 25 October 2011 09:16:54 Olof Johansson wrote: > On Tue, Oct 25, 2011 at 7:06 AM, Stephen Warren <swarren@nvidia.com> wrote: > > Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM: > >> This adds support for the embedded controller known as NVEC. The > >> driver > >> lives currently in the staging tree and we aim to promote it one level > >> higher in the near future. > >> > >> The NVEC driver uses the I2C resources of the master controller for > >> now > >> until slave support is added to the i2c-tegra driver. > > > > I'm in two minds about this: > > > > The fact that the nvec driver accesses the I2C HW directly is a hack; > > that should all happen in the I2C driver, and the nvec code should > > /just/ handle the protocol to talk to the microcontroller. > > > > To that end, we're starting to bring on some more engineers internally > > who will aim to upstream more of our downstream kernel. Part of this > > includes upstreaming the I2C slave support in a more palatable fashion. > > I know those engineers are keen in general to get started, but I can't > > yet estimate a timescale for when this particular change will be ready. > > > > That said, I'm not sure that I want to block progress on getting nvec > > and the AC100 support going. Olof, do you have any input here? Thanks. > > Yeah, let's not hold up ac100 based on not-yet-ready code, especially > since the driver is in staging. If the i2c slave driver could be done > in time to be there before nvec graduates from staging, that would > probably be ideal. Thanks Olof! > That being said, let's do it reasonably clean -- Marc, if you guys are > up for trying to define device-tree bindings for this that would be > awesome, and move to that for driver probing. That avoids adding an > include/linux/platform_data header file. I'm just checking what's the best way to do this. But first, something else: booting with device tree enabled gives me the wrong order of the mmc's again (or wrong oder of sdhci initialization). Also I only need controller 4 and 1 (in that order), how to disable 2 and 3? Same for the i2c3 controller, it should not be initialized at all or it will conflict with the nvec. For the nvec, if we only need the platform data, something like nvec@8a { gpio = <0xaa> } came into my mind (8a is the slave address). Later I felt this is wrong because 8a is not a memory address, and there should be the address of the slave controller instead (e.g. 7000c500), but that would require to move all of the resources to device tree: nvec@7000c500 { #address-cells = <1>; #size-cells = <0>; compatible = "nvidia,???"; reg = <0x7000C500 0x100>; interrupts = < 124 >; cock-frequency = <80000> gpio = <0xaa> slave-address = <0x8a> } Looking at i2c-tegra I don't see where the memory address is used at all. It is specified in the device tree, but the real memory resource still comes from the board file, so it seems to be incomplete. Given that the nvec is a kind of device connected to the i2c bus and has devices connected to it, something like this would also make sense: i2c@7000c500 { cock-frequency = <80000> is_slave; nvec { addr = <0x8a> gpio = <0xaa> keyboard { cell-index = <0> } mouse { cell-index = <0> } power { /* for AC */ cell-index = <0> } power { /* for battery */ cell-index = <1> } leds { cell-index = <0> } } } But this is more like it should look like after slave support is added to i2c- tegra. I'm a device tree newbie, so any idea what's best? And finally, looks like linux-next does a minute's silence before booting up now. Is this a known issue? Marc -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Marc Dietrich wrote at Tuesday, October 25, 2011 3:04 PM: > On Tuesday 25 October 2011 09:16:54 Olof Johansson wrote: > > On Tue, Oct 25, 2011 at 7:06 AM, Stephen Warren <swarren@nvidia.com> wrote: > > > Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM: > > >> This adds support for the embedded controller known as NVEC. The > > >> driver > > >> lives currently in the staging tree and we aim to promote it one level > > >> higher in the near future. > > >> > > >> The NVEC driver uses the I2C resources of the master controller for > > >> now > > >> until slave support is added to the i2c-tegra driver. > > > > > > I'm in two minds about this: ... > > Yeah, let's not hold up ac100 based on not-yet-ready code, especially > > since the driver is in staging. If the i2c slave driver could be done > > in time to be there before nvec graduates from staging, that would > > probably be ideal. > > Thanks Olof! > > > That being said, let's do it reasonably clean -- Marc, if you guys are > > up for trying to define device-tree bindings for this that would be > > awesome, and move to that for driver probing. That avoids adding an > > include/linux/platform_data header file. > > I'm just checking what's the best way to do this. > > But first, something else: booting with device tree enabled gives me the wrong > order of the mmc's again (or wrong oder of sdhci initialization). Also I only > need controller 4 and 1 (in that order), how to disable 2 and 3? To disable devices, add property status = "disabled". I'm not sure yet hw to solve the ordering issue. Grant had mentioned using an aliases node to help with this, but I haven't had a chance to look into that and see how it'd work. > For the nvec, if we only need the platform data, something like > > nvec@8a { > gpio = <0xaa> > } > > came into my mind (8a is the slave address). Later I felt this is wrong > because 8a is not a memory address, and there should be the address of the > slave controller instead (e.g. 7000c500), but that would require to move all > of the resources to device tree: > > nvec@7000c500 { > #address-cells = <1>; > #size-cells = <0>; > compatible = "nvidia,???"; > reg = <0x7000C500 0x100>; > interrupts = < 124 >; > cock-frequency = <80000> > gpio = <0xaa> > slave-address = <0x8a> > } Yes, that looks more correct; the address in the node's name should be where that node exists on the bus that is its parent; the 0x8a address is essentially some internal implementation detail in this case, since it's a slave. > Looking at i2c-tegra I don't see where the memory address is used at all. It > is specified in the device tree, but the real memory resource still comes from > the board file, so it seems to be incomplete. The DT reg property is converted to platform resources during DT parsing/probing, so it looks like the driveris using APIs to get resources from the board data and devices.c, but it's actually getting the data from DT. > Given that the nvec is a kind of device connected to the i2c bus and has > devices connected to it, something like this would also make sense: > > i2c@7000c500 { > cock-frequency = <80000> > is_slave; > > nvec { > addr = <0x8a> > gpio = <0xaa> > > keyboard { > cell-index = <0> > } > mouse { > cell-index = <0> > } > power { /* for AC */ > cell-index = <0> > } > power { /* for battery */ > cell-index = <1> > } > leds { > cell-index = <0> > } > > } > } > > But this is more like it should look like after slave support is added to i2c- > tegra. > > I'm a device tree newbie, so any idea what's best? I'd go with the nvec@7000c500 a little above for now. The correct binding once the I2C driver is fixed needs a little more though (well, at least I need to think more; it may already be obvious to those more experienced with DT!) > And finally, looks like linux-next does a minute's silence before booting up > now. Is this a known issue? That's odd; I certainly haven't noticed it, but I haven't pulled in the latest linux-next or a half week or so.
diff --git a/arch/arm/mach-tegra/board-paz00-pinmux.c b/arch/arm/mach-tegra/board-paz00-pinmux.c index fb20894..0a0e27a 100644 --- a/arch/arm/mach-tegra/board-paz00-pinmux.c +++ b/arch/arm/mach-tegra/board-paz00-pinmux.c @@ -154,6 +154,7 @@ static struct tegra_gpio_table gpio_table[] = { { .gpio = TEGRA_WIFI_PWRN, .enable = true }, { .gpio = TEGRA_WIFI_RST, .enable = true }, { .gpio = TEGRA_WIFI_LED, .enable = true }, + { .gpio = TEGRA_NVEC_REQ, .enable = true }, }; void paz00_pinmux_init(void) diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c index 602f8dd..3f46b37 100644 --- a/arch/arm/mach-tegra/board-paz00.c +++ b/arch/arm/mach-tegra/board-paz00.c @@ -44,6 +44,8 @@ #include "devices.h" #include "gpio-names.h" +#include "../../../drivers/staging/nvec/nvec.h" + static struct plat_serial8250_port debug_uart_platform_data[] = { { /* serial port on JP1 */ @@ -114,6 +116,11 @@ static struct platform_device leds_gpio = { }, }; +static struct nvec_platform_data nvec_pdata = { + .i2c_addr = 0x8a, + .gpio = TEGRA_NVEC_REQ, +}; + static struct platform_device *paz00_devices[] __initdata = { &debug_uart, &tegra_sdhci_device4, @@ -127,6 +134,10 @@ static void paz00_i2c_init(void) platform_device_register(&tegra_i2c_device1); platform_device_register(&tegra_i2c_device2); platform_device_register(&tegra_i2c_device4); + + tegra_i2c_device3.name = "nvec"; + tegra_i2c_device3.dev.platform_data = &nvec_pdata; + platform_device_register(&tegra_i2c_device3); } static void paz00_usb_init(void) diff --git a/arch/arm/mach-tegra/board-paz00.h b/arch/arm/mach-tegra/board-paz00.h index 2dc1899..7e978f3 100644 --- a/arch/arm/mach-tegra/board-paz00.h +++ b/arch/arm/mach-tegra/board-paz00.h @@ -32,6 +32,9 @@ #define TEGRA_WIFI_RST TEGRA_GPIO_PD1 #define TEGRA_WIFI_LED TEGRA_GPIO_PD0 +/* EC */ +#define TEGRA_NVEC_REQ TEGRA_GPIO_PV2 + void paz00_pinmux_init(void); #endif
This adds support for the embedded controller known as NVEC. The driver lives currently in the staging tree and we aim to promote it one level higher in the near future. The NVEC driver uses the I2C resources of the master controller for now until slave support is added to the i2c-tegra driver. Signed-off-by: Marc Dietrich <marvin24@gmx.de> --- arch/arm/mach-tegra/board-paz00-pinmux.c | 1 + arch/arm/mach-tegra/board-paz00.c | 11 +++++++++++ arch/arm/mach-tegra/board-paz00.h | 3 +++ 3 files changed, 15 insertions(+), 0 deletions(-)