Add NVIDIA Tegra FUSE driver

drivers/misc/fuse/Makefile line 1

Quoted 7 lines · Thierry Reding

drivers/misc/fuse/Makefile new file +1 -0

new file mode 100644
index 000000000000..0679c4febc89
@@ -0,0 +1 @@
+obj-$(CONFIG_ARCH_TEGRA)	+= tegra/

I just happened to notice this change on linux-next introduced by this patch? It's a leftover from early version of the patch, I guess?

From: Thierry Reding <[email protected]>

This commit converts the PMC support code to a platform driver. Because the boot process needs to cal into this driver very early, also set up a

Quoted 4 lines · Thierry Reding

From: Thierry Reding <[email protected]>

This commit converts the PMC support code to a platform driver. Because the boot process needs to cal into this driver very early, also set up a

Small typo: 'call' :)

include/linux/tegra-soc.h +14 -0

+/*
+ * PMC
+ */
+enum tegra_suspend_mode {
+       TEGRA_SUSPEND_NONE = 0,
+       TEGRA_SUSPEND_LP2, /* CPU voltage off */
+       TEGRA_SUSPEND_LP1, /* CPU voltage off, DRAM self-refresh */
+       TEGRA_SUSPEND_LP0, /* CPU + core voltage off, DRAM self-refresh */
+       TEGRA_MAX_SUSPEND_MODE,
+};
+
+#ifdef CONFIG_PM_SLEEP
+enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void);
+void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode);

Quoted 19 lines · Thierry Reding

include/linux/tegra-soc.h +19 -0

+/*
+ * PMC
+ */
+enum tegra_suspend_mode {
+       TEGRA_SUSPEND_NONE = 0,
+       TEGRA_SUSPEND_LP2, /* CPU voltage off */
+       TEGRA_SUSPEND_LP1, /* CPU voltage off, DRAM self-refresh */
+       TEGRA_SUSPEND_LP0, /* CPU + core voltage off, DRAM self-refresh */
+       TEGRA_MAX_SUSPEND_MODE,
+};
+
+#ifdef CONFIG_PM_SLEEP
+enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void);
+void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode);
+void tegra_pmc_enter_suspend_mode(enum tegra_suspend_mode mode);
+
+bool tegra_pmc_cpu_is_powered(int cpuid);
+int tegra_pmc_cpu_power_on(int cpuid);
+int tegra_pmc_cpu_remove_clamping(int cpuid);

include/linux/tegra-soc.h int tegra_fuse_readl(unsigned long offset, u32 *value); · line 88

Quoted 5 lines

include/linux/tegra-soc.h +5 -0

+
+void tegra_pmc_restart(enum reboot_mode mode, const char *cmd);
+#endif
+
+/*

This part is causing multiple build failures in the randconfig tests. You can avoid them by removing the #ifdef.

On a more general note, why are you adding this stuff into a global header file in the first place? All users are in the same directory in which the functions are defined.

Quoted 20 lines · Arnd Bergmann

Quoted 19 lines · Thierry Reding

(quoted diff) +19 -0

+/*
+ * PMC
+ */
+enum tegra_suspend_mode {
+       TEGRA_SUSPEND_NONE = 0,
+       TEGRA_SUSPEND_LP2, /* CPU voltage off */
+       TEGRA_SUSPEND_LP1, /* CPU voltage off, DRAM self-refresh */
+       TEGRA_SUSPEND_LP0, /* CPU + core voltage off, DRAM self-refresh */
+       TEGRA_MAX_SUSPEND_MODE,
+};
+
+#ifdef CONFIG_PM_SLEEP
+enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void);
+void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode);
+void tegra_pmc_enter_suspend_mode(enum tegra_suspend_mode mode);
+
+bool tegra_pmc_cpu_is_powered(int cpuid);
+int tegra_pmc_cpu_power_on(int cpuid);
+int tegra_pmc_cpu_remove_clamping(int cpuid);

Quoted 8 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+
+void tegra_pmc_restart(enum reboot_mode mode, const char *cmd);
+#endif
+
+/*

This part is causing multiple build failures in the randconfig tests. You can avoid them by removing the #ifdef.

How is this causing build failures? I only see them used wherever CONFIG_PM_SLEEP is defined.

Although I guess tegra-pmc.c could cause sparse warnings since it implements these functions regardless of CONFIG_PM_SLEEP, which probably is the bug that should be fixed.

Do you have a randconfig that I can use to reproduce this and come up with a fix?

Quoted 3 lines

On a more general note, why are you adding this stuff into a global header file in the first place? All users are in the same directory in which the functions are defined.

That's mostly preparatory work. We'll need to move tegra-pmc.c out of arch/arm/mach-tegra at some point. I've proposed two patches already to do that, one move the driver to drivers/soc/tegra and was massively NAK'ed (I'm still not sure I agree) and people requested that this be moved into drivers/power. I then posted a 2 patch series to move power supply drivers into a subdirectory (drivers/power/supply) so that drivers in drivers/power didn't have to depend on CONFIG_POWER_SUPPLY for no good reason.

The latter series didn't receive any comments whatsoever in over a week, so in order to keep things moving forward I respun the PMC patch to do the conversion without moving the code out of arch/arm/mach-tegra yet. That way moving the driver out of arch/arm/mach-tegra will be a simple matter of moving and the rework will already be done. You were Cc'ed on the second series, so if you want to take a look (and maybe help get things moving) the patches are called:

[PATCH 0/2] Restructure driver/power and add Tegra PMC driver [PATCH 1/2] power: Move power-supply drivers to subdirectory [PATCH 2/2] ARM: tegra: Convert PMC to a driver

Quoted 33 lines · On Wednesday 16 July 2014 15:22:41 Thierry Reding wrote:

Quoted 20 lines · Arnd Bergmann

Quoted 19 lines · Thierry Reding

(quoted diff) +19 -0

+/*
+ * PMC
+ */
+enum tegra_suspend_mode {
+       TEGRA_SUSPEND_NONE = 0,
+       TEGRA_SUSPEND_LP2, /* CPU voltage off */
+       TEGRA_SUSPEND_LP1, /* CPU voltage off, DRAM self-refresh */
+       TEGRA_SUSPEND_LP0, /* CPU + core voltage off, DRAM self-refresh */
+       TEGRA_MAX_SUSPEND_MODE,
+};
+
+#ifdef CONFIG_PM_SLEEP
+enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void);
+void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode);
+void tegra_pmc_enter_suspend_mode(enum tegra_suspend_mode mode);
+
+bool tegra_pmc_cpu_is_powered(int cpuid);
+int tegra_pmc_cpu_power_on(int cpuid);
+int tegra_pmc_cpu_remove_clamping(int cpuid);

Quoted 8 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+
+void tegra_pmc_restart(enum reboot_mode mode, const char *cmd);
+#endif
+
+/*

This part is causing multiple build failures in the randconfig tests. You can avoid them by removing the #ifdef.

How is this causing build failures? I only see them used wherever CONFIG_PM_SLEEP is defined.

Quoted 6 lines

Although I guess tegra-pmc.c could cause sparse warnings since it implements these functions regardless of CONFIG_PM_SLEEP, which probably is the bug that should be fixed.

Do you have a randconfig that I can use to reproduce this and come up with a fix?

I got this error (always):

/git/arm-soc/arch/arm/mach-tegra/tegra.c:165:13: error: 'tegra_pmc_restart' undeclared here (not in a function)
.restart = tegra_pmc_restart,

             ^

make[3]: *** [arch/arm/mach-tegra/tegra.o] Error 1

when CONFIG_PM_SLEEP is disabled, and also this one when SMP is turned on in addition:

/git/arm-soc/arch/arm/mach-tegra/platsmp.c: In function 'tegra30_boot_secondary':
/git/arm-soc/arch/arm/mach-tegra/platsmp.c:97:4: error: implicit declaration of function 'tegra_pmc_cpu_is_powered' [-Werror=implicit-function-declaration]

    if (tegra_pmc_cpu_is_powered(cpu))
    ^
/git/arm-soc/arch/arm/mach-tegra/platsmp.c:110:3: error: implicit declaration of function 'tegra_pmc_cpu_power_on' [-Werror=implicit-function-declaration]
   ret = tegra_pmc_cpu_power_on(cpu);
   ^

/git/arm-soc/arch/arm/mach-tegra/platsmp.c:129:2: error: implicit declaration of function 'tegra_pmc_cpu_remove_clamping' [-Werror=implicit-function-declaration]
ret = tegra_pmc_cpu_remove_clamping(cpu);
^
cc1: some warnings being treated as errors

I applied this hack locally to work around that:

include/linux/tegra-soc.h +0 -1

index 70a612442cda..a89fe9e588ac 100644
@@ -73,7 +73,6 @@ enum tegra_suspend_mode {
 	TEGRA_MAX_SUSPEND_MODE,
 };
 
-#ifdef CONFIG_PM_SLEEP
 enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void);
 void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode);
 void tegra_pmc_enter_suspend_mode(enum tegra_suspend_mode mode);

Quoted 22 lines

Quoted 4 lines

On a more general note, why are you adding this stuff into a global header file in the first place? All users are in the same directory in which the functions are defined.

That's mostly preparatory work. We'll need to move tegra-pmc.c out of arch/arm/mach-tegra at some point. I've proposed two patches already to do that, one move the driver to drivers/soc/tegra and was massively NAK'ed (I'm still not sure I agree) and people requested that this be moved into drivers/power. I then posted a 2 patch series to move power supply drivers into a subdirectory (drivers/power/supply) so that drivers in drivers/power didn't have to depend on CONFIG_POWER_SUPPLY for no good reason.

The latter series didn't receive any comments whatsoever in over a week, so in order to keep things moving forward I respun the PMC patch to do the conversion without moving the code out of arch/arm/mach-tegra yet. That way moving the driver out of arch/arm/mach-tegra will be a simple matter of moving and the rework will already be done. You were Cc'ed on the second series, so if you want to take a look (and maybe help get things moving) the patches are called:

Quoted 3 lines

[PATCH 0/2] Restructure driver/power and add Tegra PMC driver [PATCH 1/2] power: Move power-supply drivers to subdirectory [PATCH 2/2] ARM: tegra: Convert PMC to a driver

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 5 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

Quoted 6 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 5 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

Quoted 7 lines · Arnd Bergmann

Quoted 6 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 5 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 8 lines

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

I completely agree. However spreading the SoC-specific functions across multiple header files isn't going to help. If we keep all the per-vendor APIs in one file it makes it easier to see what could still be moved off into a separate subsystem.

Now for PMC specifically, we've investigated converting the powergate API to power domains. I don't think it will be possible to make that work. The issue is that there's a defined sequence that needs to be respected to make sure the device is powered up properly. That sequence involves the primary clock and reset of the device. It's been proposed to make these clocks available to the PMC driver so that it can control them, but then we can't make sure that clocks are really off if they need to be, since we have two drivers accessing them. The only way I see to make that work reliably is by moving complete control of the powergate into drivers so that they can make sure clocks and resets are in the correct states.

The PMC driver also provides access to I/O rails and specifically a deep power down state. Some modules are in deep power down state by default, so they need to be brought out of that state. I suppose this would be easier to turn into a generic framework because there aren't any cross- dependencies like for powergates, but I'm not aware of any other SoC having a similar feature (or implementation thereof in the kernel). And adding a subsystem just for the sake of it if only one implementation is available isn't a good idea in my opinion because it will be naturally designed to work best (and therefore maybe only) for the one instance.

This issue is a fundamental one and there are bound to be other SoCs that have similarly unique blocks for which it's impractical to add a framework. I suspect the primary reason why we haven't run into it this frequently is because a lot of it is still hidden in arch/arm/mach-*.

I'm open to suggestions of course, but the best option I currently see is to collect these custom APIs in a central place so that we can easily compare various SoCs for commonalities as time goes by and factor them out into subsystems where appropriate.

For the same reason I think it's valid to put this type of code into drivers/soc. That way we have one subdirectory to look through for potential unification rather than various ones sprinkled across arch directories. It makes little sense in my opinion to move this code to drivers/power if there's no common framework anyway.

Quoted 22 lines · Thierry Reding

Quoted 7 lines · Arnd Bergmann

Quoted 6 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 5 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 8 lines

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

I completely agree. However spreading the SoC-specific functions across multiple header files isn't going to help. If we keep all the per-vendor APIs in one file it makes it easier to see what could still be moved off into a separate subsystem.

Quoted 11 lines

Now for PMC specifically, we've investigated converting the powergate API to power domains. I don't think it will be possible to make that work. The issue is that there's a defined sequence that needs to be respected to make sure the device is powered up properly. That sequence involves the primary clock and reset of the device. It's been proposed to make these clocks available to the PMC driver so that it can control them, but then we can't make sure that clocks are really off if they need to be, since we have two drivers accessing them. The only way I see to make that work reliably is by moving complete control of the powergate into drivers so that they can make sure clocks and resets are in the correct states.

Quoted 20 lines

The PMC driver also provides access to I/O rails and specifically a deep power down state. Some modules are in deep power down state by default, so they need to be brought out of that state. I suppose this would be easier to turn into a generic framework because there aren't any cross- dependencies like for powergates, but I'm not aware of any other SoC having a similar feature (or implementation thereof in the kernel). And adding a subsystem just for the sake of it if only one implementation is available isn't a good idea in my opinion because it will be naturally designed to work best (and therefore maybe only) for the one instance.

This issue is a fundamental one and there are bound to be other SoCs that have similarly unique blocks for which it's impractical to add a framework. I suspect the primary reason why we haven't run into it this frequently is because a lot of it is still hidden in arch/arm/mach-*.

I'm open to suggestions of course, but the best option I currently see is to collect these custom APIs in a central place so that we can easily compare various SoCs for commonalities as time goes by and factor them out into subsystems where appropriate.

Quoted 5 lines

For the same reason I think it's valid to put this type of code into drivers/soc. That way we have one subdirectory to look through for potential unification rather than various ones sprinkled across arch directories. It makes little sense in my opinion to move this code to drivers/power if there's no common framework anyway.

I agree. We can move them out and make them common them later if needed.

We're sometimes trying too hard to find proper homes for various new drivers, which means that we're proliferating the kernel with a lot of new driver directories that have only one or two drivers in them.

I'd rather collect stuff in drivers/soc, and move it out as needed later. Especially since we merge drivers/soc through one merge path (arm-soc) and can keep an eye on it, while the scatter-drivers-everywhere approach merges through various maintainers.

Quoted 24 lines · On Wednesday 16 July 2014 12:34:56 Olof Johansson wrote:

Quoted 22 lines · Thierry Reding

Quoted 7 lines · Arnd Bergmann

Quoted 6 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 5 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 8 lines

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

I completely agree. However spreading the SoC-specific functions across multiple header files isn't going to help. If we keep all the per-vendor APIs in one file it makes it easier to see what could still be moved off into a separate subsystem.

Quoted 11 lines

Quoted 11 lines

Now for PMC specifically, we've investigated converting the powergate API to power domains. I don't think it will be possible to make that work. The issue is that there's a defined sequence that needs to be respected to make sure the device is powered up properly. That sequence involves the primary clock and reset of the device. It's been proposed to make these clocks available to the PMC driver so that it can control them, but then we can't make sure that clocks are really off if they need to be, since we have two drivers accessing them. The only way I see to make that work reliably is by moving complete control of the powergate into drivers so that they can make sure clocks and resets are in the correct states.

I don't completely follow, but that's ok ;-)

Quoted 32 lines

Quoted 20 lines

The PMC driver also provides access to I/O rails and specifically a deep power down state. Some modules are in deep power down state by default, so they need to be brought out of that state. I suppose this would be easier to turn into a generic framework because there aren't any cross- dependencies like for powergates, but I'm not aware of any other SoC having a similar feature (or implementation thereof in the kernel). And adding a subsystem just for the sake of it if only one implementation is available isn't a good idea in my opinion because it will be naturally designed to work best (and therefore maybe only) for the one instance.

This issue is a fundamental one and there are bound to be other SoCs that have similarly unique blocks for which it's impractical to add a framework. I suspect the primary reason why we haven't run into it this frequently is because a lot of it is still hidden in arch/arm/mach-*.

I'm open to suggestions of course, but the best option I currently see is to collect these custom APIs in a central place so that we can easily compare various SoCs for commonalities as time goes by and factor them out into subsystems where appropriate.

Quoted 5 lines

For the same reason I think it's valid to put this type of code into drivers/soc. That way we have one subdirectory to look through for potential unification rather than various ones sprinkled across arch directories. It makes little sense in my opinion to move this code to drivers/power if there's no common framework anyway.

I agree. We can move them out and make them common them later if needed.

We're sometimes trying too hard to find proper homes for various new drivers, which means that we're proliferating the kernel with a lot of new driver directories that have only one or two drivers in them.

Quoted 5 lines

I'd rather collect stuff in drivers/soc, and move it out as needed later. Especially since we merge drivers/soc through one merge path (arm-soc) and can keep an eye on it, while the scatter-drivers-everywhere approach merges through various maintainers.

Ok. I'm fine with having one driver in drivers/soc for the pmc (and a few associated bits if necessary) and a header file for that. If you end up with two separate drivers in drivers/soc, I'd also prefer two separate header files.

It may be a good idea to put these headers somewhere other than include/linux/*.h, which is completely overloaded by random stuff. We could use include/linux/soc/*.h or include/soc/*.h for those.

We could go all the way and make it include/soc/tegra/*.h for better namespacing. I guess either way would be fine, really, since the number of files in those directories should be small by definition, so we should be able to do without the extra SoC directory, too. I have a slight preference for a separate SoC directory, do you have any objections?

Quoted 7 lines · On Thursday 17 July 2014 13:06:53 Thierry Reding wrote:

We could go all the way and make it include/soc/tegra/*.h for better namespacing. I guess either way would be fine, really, since the number of files in those directories should be small by definition, so we should be able to do without the extra SoC directory, too. I have a slight preference for a separate SoC directory, do you have any objections?

I'm fine with it either way. I just noticed that you have now moved the file, which resulted in a build error:

../drivers/ata/ahci_tegra.c:27:35: fatal error: linux/tegra-powergate.h: No such file or directory

 #include <linux/tegra-powergate.h>
                                   ^

compilation terminated.
make[4]: *** [drivers/ata/ahci_tegra.o] Error 1

so somebody needs to pick up this patch:

drivers/ata/ahci_tegra.c +1 -1

index d30bb21afd67..d7c6b1f550cd 100644
@@ -24,8 +24,8 @@
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
-#include <linux/tegra-powergate.h>
 #include <linux/regulator/consumer.h>
+#include <soc/tegra/pmc.h>
 #include "ahci.h"
 
 #define SATA_CONFIGURATION_0				0x180

I haven't checked which trees are affected of if you have already posted a patch to do this.

Arnd

Adding Tejun, since he took this into his tree on Friday I think, just a few hours after I sent out pull requests for this.

I hadn't considered this build dependency. We could resolve this in two ways I think:

- merge the for-3.17/fuse-move branch from the Tegra tree into the libata tree and apply Arnd's patch to the libata tree

Quoted 3 lines · Thierry Reding

Adding Tejun, since he took this into his tree on Friday I think, just a few hours after I sent out pull requests for this.

Quoted 5 lines

I hadn't considered this build dependency. We could resolve this in two ways I think:

- merge the for-3.17/fuse-move branch from the Tegra tree into the libata tree and apply Arnd's patch to the libata tree

I can pull the tegra tree into the libata if the tegra maintainer is okay with it.

* PGP Signed by an unknown key

Quoted 7 lines · Arnd Bergmann

Quoted 6 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 5 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 8 lines

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

I completely agree. However spreading the SoC-specific functions across multiple header files isn't going to help. If we keep all the per-vendor APIs in one file it makes it easier to see what could still be moved off into a separate subsystem.

Now for PMC specifically, we've investigated converting the powergate API to power domains. I don't think it will be possible to make that work. The issue is that there's a defined sequence that needs to be respected to make sure the device is powered up properly. That sequence involves the primary clock and reset of the device. It's been proposed to make these clocks available to the PMC driver so that it can control them, but then we can't make sure that clocks are really off if they need to be, since we have two drivers accessing them. The only way I see

to make that work reliably is by moving complete control of the powergate into drivers so that they can make sure clocks and resets are in the correct states.

Quoted 24 lines · Thierry Reding

* PGP Signed by an unknown key

Quoted 7 lines · Arnd Bergmann

Quoted 6 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 5 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 8 lines

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

I completely agree. However spreading the SoC-specific functions across multiple header files isn't going to help. If we keep all the per-vendor APIs in one file it makes it easier to see what could still be moved off into a separate subsystem.

Quoted 8 lines

Now for PMC specifically, we've investigated converting the powergate API to power domains. I don't think it will be possible to make that work. The issue is that there's a defined sequence that needs to be respected to make sure the device is powered up properly. That sequence involves the primary clock and reset of the device. It's been proposed to make these clocks available to the PMC driver so that it can control them, but then we can't make sure that clocks are really off if they need to be, since we have two drivers accessing them. The only way I see

resets do not have reference counts, so they can be controlled by a powerdomain driver without any problems. For clocks, there would only be a problem for the module clocks if the drivers don't use runtime PM. If we move all drivers to runtime PM, the clock control can move into the powerdomain code and runtime PM will ensure domains are not turned off with active modules.

Quoted 4 lines

to make that work reliably is by moving complete control of the powergate into drivers so that they can make sure clocks and resets are in the correct states.

Which won't work if you have domains which contain several modules.

Quoted 46 lines · Peter De Schrijver

Quoted 24 lines · Thierry Reding

* PGP Signed by an unknown key

Quoted 7 lines · Arnd Bergmann

Quoted 6 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 5 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 8 lines

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

I completely agree. However spreading the SoC-specific functions across multiple header files isn't going to help. If we keep all the per-vendor APIs in one file it makes it easier to see what could still be moved off into a separate subsystem.

Quoted 8 lines

Now for PMC specifically, we've investigated converting the powergate API to power domains. I don't think it will be possible to make that work. The issue is that there's a defined sequence that needs to be respected to make sure the device is powered up properly. That sequence involves the primary clock and reset of the device. It's been proposed to make these clocks available to the PMC driver so that it can control them, but then we can't make sure that clocks are really off if they need to be, since we have two drivers accessing them. The only way I see

resets do not have reference counts, so they can be controlled by a powerdomain driver without any problems. For clocks, there would only be a problem for the module clocks if the drivers don't use runtime PM. If we move all drivers to runtime PM, the clock control can move into the powerdomain code and runtime PM will ensure domains are not turned off with active modules.

Quoted 4 lines

to make that work reliably is by moving complete control of the powergate into drivers so that they can make sure clocks and resets are in the correct states.

Quoted 1 line

Which won't work if you have domains which contain several modules.

We also need to control the memory clients in the domains using MC_CLIENT_HOTRESET_CTRL.

* PGP Signed by an unknown key

Quoted 36 lines · Peter De Schrijver

Quoted 35 lines · Peter De Schrijver

Quoted 19 lines · Thierry Reding

Quoted 1 line

Old Signed by an unknown key

Quoted 5 lines · Arnd Bergmann

Quoted 4 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 4 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 7 lines

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

I completely agree. However spreading the SoC-specific functions across multiple header files isn't going to help. If we keep all the per-vendor APIs in one file it makes it easier to see what could still be moved off into a separate subsystem.

Quoted 8 lines

Now for PMC specifically, we've investigated converting the powergate API to power domains. I don't think it will be possible to make that work. The issue is that there's a defined sequence that needs to be respected to make sure the device is powered up properly. That sequence involves the primary clock and reset of the device. It's been proposed to make these clocks available to the PMC driver so that it can control them, but then we can't make sure that clocks are really off if they need to be, since we have two drivers accessing them. The only way I see

resets do not have reference counts, so they can be controlled by a powerdomain driver without any problems. For clocks, there would only be a problem for the module clocks if the drivers don't use runtime PM. If we move all drivers to runtime PM, the clock control can move into the powerdomain code and runtime PM will ensure domains are not turned off with active modules.

Quoted 7 lines

Quoted 5 lines

Quoted 4 lines

to make that work reliably is by moving complete control of the powergate into drivers so that they can make sure clocks and resets are in the correct states.

Which won't work if you have domains which contain several modules.

We also need to control the memory clients in the domains using MC_CLIENT_HOTRESET_CTRL.

Oh, great. More interdependencies...

Quoted 43 lines · Thierry Reding

* PGP Signed by an unknown key

Quoted 40 lines · Peter De Schrijver

Quoted 39 lines · Peter De Schrijver

Quoted 19 lines · Thierry Reding

Quoted 1 line

Old Signed by an unknown key

Quoted 5 lines · Arnd Bergmann

Quoted 4 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 4 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 7 lines

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

I completely agree. However spreading the SoC-specific functions across multiple header files isn't going to help. If we keep all the per-vendor APIs in one file it makes it easier to see what could still be moved off into a separate subsystem.

Quoted 8 lines

Now for PMC specifically, we've investigated converting the powergate API to power domains. I don't think it will be possible to make that work. The issue is that there's a defined sequence that needs to be respected to make sure the device is powered up properly. That sequence involves the primary clock and reset of the device. It's been proposed to make these clocks available to the PMC driver so that it can control them, but then we can't make sure that clocks are really off if they need to be, since we have two drivers accessing them. The only way I see

resets do not have reference counts, so they can be controlled by a powerdomain driver without any problems. For clocks, there would only be a problem for the module clocks if the drivers don't use runtime PM. If we move all drivers to runtime PM, the clock control can move into the powerdomain code and runtime PM will ensure domains are not turned off with active modules.

Quoted 4 lines

to make that work reliably is by moving complete control of the powergate into drivers so that they can make sure clocks and resets are in the correct states.

Quoted 4 lines

Quoted 3 lines

Quoted 1 line

Which won't work if you have domains which contain several modules.

We also need to control the memory clients in the domains using MC_CLIENT_HOTRESET_CTRL.

Oh, great. More interdependencies...

Some domains depend on others. Can we take this into account?

* PGP Signed by an unknown key

Quoted 43 lines · Vince Hsu

Quoted 42 lines · Thierry Reding

Quoted 1 line

Old Signed by an unknown key

Quoted 40 lines · Peter De Schrijver

Quoted 39 lines · Peter De Schrijver

Quoted 19 lines · Thierry Reding

Quoted 1 line

Old Signed by an unknown key

Quoted 5 lines · Arnd Bergmann

Quoted 4 lines · On Wednesday 16 July 2014 17:14:29 Thierry Reding wrote:

Quoted 4 lines

Ok, I'll have a look. I think when this becomes a separate driver, it should also have its own header file, so maybe you can in the meantime make it a local header file in mach-tegra until we have found a good place for it.

Quoted 7 lines

Quoted 4 lines

Why do you think it should be a separate header? We already have a couple in include/linux and I'm not sure it's useful to add even more. If anything I would've thought it made sense to move the content of the other headers into tegra-soc.h.

I very much dislike the idea of having a per-vendor header file that everything gets crammed into. We should try to have proper subsystems and generic interfaces for these wherever possible.

I completely agree. However spreading the SoC-specific functions across multiple header files isn't going to help. If we keep all the per-vendor APIs in one file it makes it easier to see what could still be moved off into a separate subsystem.

Quoted 8 lines

Now for PMC specifically, we've investigated converting the powergate API to power domains. I don't think it will be possible to make that work. The issue is that there's a defined sequence that needs to be respected to make sure the device is powered up properly. That sequence involves the primary clock and reset of the device. It's been proposed to make these clocks available to the PMC driver so that it can control them, but then we can't make sure that clocks are really off if they need to be, since we have two drivers accessing them. The only way I see

resets do not have reference counts, so they can be controlled by a powerdomain driver without any problems. For clocks, there would only be a problem for the module clocks if the drivers don't use runtime PM. If we move all drivers to runtime PM, the clock control can move into the powerdomain code and runtime PM will ensure domains are not turned off with active modules.

Quoted 4 lines

to make that work reliably is by moving complete control of the powergate into drivers so that they can make sure clocks and resets are in the correct states.

Quoted 5 lines

Quoted 4 lines

Quoted 3 lines

Quoted 1 line

Which won't work if you have domains which contain several modules.

We also need to control the memory clients in the domains using MC_CLIENT_HOTRESET_CTRL.

Oh, great. More interdependencies...

Some domains depend on others. Can we take this into account?

I'm not aware of any dependencies. Can you point me at the relevant section in the TRM?

From: Thierry Reding <[email protected]>

This series is an extension of Peter's original series to add a proper driver for the FUSE block on Tegra.

Patches 1 and 2 are preparatory work that cleans up include lists and introduces a function to query the Tegra chip ID rather than accessing a simple variable. This is used by subsequent patches to allow us to execute code when the variable is accessed to help in pin-pointing locations where it's accessed before the driver had a chance to initialize.

Patches 3 through 8 are Peter's series with a fixup patch by Stephen. I also moved the driver to drivers/soc/tegra/fuse as requested by Olof.

Patches 9 and 10 defer usages of the tegra_get_chip_id() function to a later stage (pure_initcall). This allows patch 11 to set up the early FUSE support code from an early initcall. This has the advantage of not requiring an explicit call from SoC setup code in arch/arm/mach-tegra and will allow the code to be shared on 64-bit variants of Tegra.

Patch 12 finally turns the PMC and powergate support code into a proper driver which also sets up a minimal environment from an early initcall. The driver isn't moved out of arch/arm/mach-tegra yet because people have suggested drivers/power as a good home, but that whole directory depends on the POWER_SUPPLY Kconfig symbol yet this driver doesn't have anything to do with that. Once that debate has been settled the driver can easily be moved out in a separate patch.

Some of the patches in this series introduce diagnostic WARN() messages if functions are called without setup having completed. I've booted the v3.16-rc1 kernel with these changes on top on Tegra20 (TrimSlice), Tegra30 (Beaver), Tegra114 (Dalmore) and Tegra124 (Jetson TK1) without encountering any of the diagnostic warnings and without noticing any breakage.

Olof, this series should address the concerns you expressed after reviewing Stephen's earlier pull request for Peter's FUSE driver series. It would be great if you could take another look to see if this is more according to your taste. I'll see if I can take this through linux-next for a little and if you have no objections will submit another pull request next week.

Peter De Schrijver (5):
ARM: tegra: export apb dma readl/writel

Quoted 36 lines · Thierry Reding

From: Thierry Reding <[email protected]>

This series is an extension of Peter's original series to add a proper driver for the FUSE block on Tegra.

Patches 1 and 2 are preparatory work that cleans up include lists and introduces a function to query the Tegra chip ID rather than accessing a simple variable. This is used by subsequent patches to allow us to execute code when the variable is accessed to help in pin-pointing locations where it's accessed before the driver had a chance to initialize.

Patches 3 through 8 are Peter's series with a fixup patch by Stephen. I also moved the driver to drivers/soc/tegra/fuse as requested by Olof.

Patches 9 and 10 defer usages of the tegra_get_chip_id() function to a later stage (pure_initcall). This allows patch 11 to set up the early FUSE support code from an early initcall. This has the advantage of not requiring an explicit call from SoC setup code in arch/arm/mach-tegra and will allow the code to be shared on 64-bit variants of Tegra.

Patch 12 finally turns the PMC and powergate support code into a proper driver which also sets up a minimal environment from an early initcall. The driver isn't moved out of arch/arm/mach-tegra yet because people have suggested drivers/power as a good home, but that whole directory depends on the POWER_SUPPLY Kconfig symbol yet this driver doesn't have anything to do with that. Once that debate has been settled the driver can easily be moved out in a separate patch.

Some of the patches in this series introduce diagnostic WARN() messages if functions are called without setup having completed. I've booted the v3.16-rc1 kernel with these changes on top on Tegra20 (TrimSlice), Tegra30 (Beaver), Tegra114 (Dalmore) and Tegra124 (Jetson TK1) without encountering any of the diagnostic warnings and without noticing any breakage.

Quoted 6 lines

Olof, this series should address the concerns you expressed after reviewing Stephen's earlier pull request for Peter's FUSE driver series. It would be great if you could take another look to see if this is more according to your taste. I'll see if I can take this through linux-next for a little and if you have no objections will submit another pull request next week.

I haven't looked at all the patches in detail yet, but I do like the approach in general. I've also applied and pushed them out for build+test here, but they're queued behind a couple of other builds so I won't have it until morning (you can poll for boot logs at arm-soc.lixom.net/bootlogs/misc/v3.16-rc4-371-g2c9d948 if you want to preempt me).

Quoted 2 lines

Peter De Schrijver (5):
ARM: tegra: export apb dma readl/writel

I thought I saw a patch from Peter that moved the apbio read code to the fuse driver since there are no other consumers. I think that's a reasonable thing to do.