| Message ID | cover.1498642745.git.viresh.kumar@linaro.org |
|---|---|
| Headers | show |
| Series | drivers: Add boot constraints core | expand |
On 28.06.2017 10:26, Viresh Kumar wrote: Hi, > Some devices are powered ON by the bootloaders before the bootloader > handovers control to Linux. It maybe important for those devices to keep > working until the time a Linux device driver probes the device and > reconfigure its resources. Just curious: aren't the devices (at least w/ DT) only initialized after dependencies (eg. regulators) are already up ? Let's imagine a LCD panel driven by a regulator behind SPI. The panel driver would ask the regulator framework to switch on, which would call the regulator driver. This one now would talk to SPI framework, which finally calls the SPI driver. If SPI isn't up yet, it would all be deferred, leaving the panel driver uninitialized (tried again later). Am I wrong here ? If the bootloader already switched on the panel (therefore already enabled SPI), why does it matter that the panel driver isn't up yet ? Is there anything that accidentially switches it off again (eg. by resetting the regulator) ? If so, shouldn't the corresponding drivers make sure that all depencies are met before doing anyhing w/ the device, not even attempting a reset ? --mtx
On Wed, Jun 28, 2017 at 03:56:33PM +0530, Viresh Kumar wrote: > A typical example of that can be the LCD controller, which is used by > the bootloaders to show image(s) while the machine is booting into > Linux. The LCD controller can be using some resources, like clk, > regulators, etc, that are shared between several devices. These shared > resources should be programmed so that all the users of them are > satisfied. If a user (X) driver gets probed before the LCD controller > driver in this case, then it may end up reconfiguring these resources to > ranges satisfying the current users (only user X) and that can make the > LCD screen unstable. The thing that concerns me most about this is that typically the LCD controller will be performing DMA to system RAM. The location of the frame buffer is unknown to the decompressor - and as the decompressor self-relocates itself (using purely assembly code), it could relocate itself on top of the frame buffer, causing the "nice" image to become very colourful. The decompressor doesn't have the information from DT at that point to know what are safe locations, so it's up to the boot loader to place the frame buffer somewhere out of the way. (If people want to write a DT parser in position independent ARM assembly code that may change.) As long as people realise this, then it's not a problem, but given the number of problems that we've already encountered with boot loaders and memory space layout, I don't trust them to get this right. Right now, the ARM kernel booting document requires: - Quiesce all DMA capable devices so that memory does not get corrupted by bogus network packets or disk data. This will save you many hours of debug. so we would need to modify that to make an exception for LCD controllers. However, we definitely can't have devices left enabled which are capable of writing to system memory, or which changing system memory is likely to cause bad effects (eg, packet ring buffers, USB buffers etc, which is really what the above requirement is about.) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
On 29.06.2017 12:49, Russell King - ARM Linux wrote: > The location of the frame buffer is unknown to the decompressor - and > as the decompressor self-relocates itself (using purely assembly code), > it could relocate itself on top of the frame buffer, causing the "nice" > image to become very colourful. Could the bootloader pass safe (or blocked) memory regions to the decompressor ? --mtx
On 29-06-17, 12:40, Enrico Weigelt, metux IT consult wrote: > Just curious: aren't the devices (at least w/ DT) only initialized after > dependencies (eg. regulators) are already up ? No. Drivers are registered to the kernel (randomly, though we can know their order) and devices are registered separately (platform/amba devices get registered automatically with DT, hint: drivers/of/platform.c). The device core checks while registering devices/drivers if their drivers/devices are available or not. If yes, then the devices are probed using the drivers. Now the drivers must make sure all the dependencies are met at this point, else they can return -EPROBE_DEFER and the kernel will try probing them again. > Let's imagine a LCD panel driven by a regulator behind SPI. The panel > driver would ask the regulator framework to switch on, which would > call the regulator driver. This one now would talk to SPI framework, > which finally calls the SPI driver. If SPI isn't up yet, it would all > be deferred, leaving the panel driver uninitialized (tried again later). This should happen in probe, otherwise we are screwed. > If the bootloader already switched on the panel (therefore already > enabled SPI), why does it matter that the panel driver isn't up yet ? But the kernel doesn't know how it is configured, there can be so many configurable parameters. The kernel needs to do it again by itself. > Is there anything that accidentially switches it off again (eg. by > resetting the regulator) ? It is not just about switching it off, but the configuration here. Let me try with an example. A regulator is shared between LCD and DMA controller. Operable ranges of the regulator: 1.8 - 3.0 V Range required by LCD: 2.0 - 3.0 V Range required by DMA: 1.8 - 2.5 V Of course the right range for both of them to work here is 2.0 - 2.5 V. Now the LCD is already enabled by the bootloader and kernel doesn't know the ranges. DMA requests for the regulator and we set its voltage to 1.8 V. LCD is screwed while we are still booting. This will go worse if we add more devices to this example that share the same regulator. -- viresh
On 29-06-17, 13:49, Russell King - ARM Linux wrote: > The thing that concerns me most about this is that typically the LCD > controller will be performing DMA to system RAM. > > The location of the frame buffer is unknown to the decompressor - and > as the decompressor self-relocates itself (using purely assembly code), > it could relocate itself on top of the frame buffer, causing the "nice" > image to become very colourful. > > The decompressor doesn't have the information from DT at that point to > know what are safe locations, so it's up to the boot loader to place > the frame buffer somewhere out of the way. (If people want to write > a DT parser in position independent ARM assembly code that may change.) > > As long as people realise this, then it's not a problem, but given the > number of problems that we've already encountered with boot loaders and > memory space layout, I don't trust them to get this right. > > Right now, the ARM kernel booting document requires: > > - Quiesce all DMA capable devices so that memory does not get > corrupted by bogus network packets or disk data. This will save > you many hours of debug. > > so we would need to modify that to make an exception for LCD controllers. > However, we definitely can't have devices left enabled which are capable > of writing to system memory, or which changing system memory is likely > to cause bad effects (eg, packet ring buffers, USB buffers etc, which is > really what the above requirement is about.) Well, LCD was just an example here. But yeah, it is one of the most probable case we have. So, this thing is already working for sure, of course with some out of tree hacks. Every smart phone shows their company's logo (some kind of flash) while the phone boots. How do they get around such issues? @Stephen: Any idea how Qcom does it ? :) Must be fixing some area in RAM for this purpose, isn't it ? -- viresh
On 29.06.2017 14:47, Viresh Kumar wrote: > No. Drivers are registered to the kernel (randomly, though we can know > their order) and devices are registered separately (platform/amba > devices get registered automatically with DT, hint: > drivers/of/platform.c). The device core checks while registering > devices/drivers if their drivers/devices are available or not. If > yes, then the devices are probed using the drivers. Now the drivers > must make sure all the dependencies are met at this point, else they > can return -EPROBE_DEFER and the kernel will try probing them again. Could we somehow introduce an strict ordering ? Maybe by letting the device core know of the dependencies, before individual probe()'s explicitly ask for them ? >> Let's imagine a LCD panel driven by a regulator behind SPI. The panel >> driver would ask the regulator framework to switch on, which would >> call the regulator driver. This one now would talk to SPI framework, >> which finally calls the SPI driver. If SPI isn't up yet, it would all >> be deferred, leaving the panel driver uninitialized (tried again later). > > This should happen in probe, otherwise we are screwed. Yes, but the probe result may be deferred, so it's tried again in the next round. Correct ? >> If the bootloader already switched on the panel (therefore already >> enabled SPI), why does it matter that the panel driver isn't up yet ? > > But the kernel doesn't know how it is configured, there can be so many > configurable parameters. The kernel needs to do it again by itself. Could it read back the config ? By the way: I've got a similar problem w/ gpmc right now: uboot already sets it up, but the kernel only knows about one CS (for the nand) and screwes up the others (eg. fpga), so it cant access the fpga . Until I've sorted out all the parameters for DT (unfortunately, only have the raw register values), I'll have to rely on an userland test program to set it all up ... > Let me try with an example. A regulator is shared between LCD and DMA > controller. > > Operable ranges of the regulator: 1.8 - 3.0 V > Range required by LCD: 2.0 - 3.0 V > Range required by DMA: 1.8 - 2.5 V Would a config readback help here ? The regulator core then should know that we're already in proper range for DMA and no need to touch the regulator. --mtx
On Thu, Jun 29, 2017 at 08:28:08PM +0530, Viresh Kumar wrote: > On 29-06-17, 13:49, Russell King - ARM Linux wrote: > > The thing that concerns me most about this is that typically the LCD > > controller will be performing DMA to system RAM. > > > > The location of the frame buffer is unknown to the decompressor - and > > as the decompressor self-relocates itself (using purely assembly code), > > it could relocate itself on top of the frame buffer, causing the "nice" > > image to become very colourful. > > > > The decompressor doesn't have the information from DT at that point to > > know what are safe locations, so it's up to the boot loader to place > > the frame buffer somewhere out of the way. (If people want to write > > a DT parser in position independent ARM assembly code that may change.) > > > > As long as people realise this, then it's not a problem, but given the > > number of problems that we've already encountered with boot loaders and > > memory space layout, I don't trust them to get this right. > > > > Right now, the ARM kernel booting document requires: > > > > - Quiesce all DMA capable devices so that memory does not get > > corrupted by bogus network packets or disk data. This will save > > you many hours of debug. > > > > so we would need to modify that to make an exception for LCD controllers. > > However, we definitely can't have devices left enabled which are capable > > of writing to system memory, or which changing system memory is likely > > to cause bad effects (eg, packet ring buffers, USB buffers etc, which is > > really what the above requirement is about.) > > Well, LCD was just an example here. But yeah, it is one of the most > probable case we have. > > So, this thing is already working for sure, of course with some out of > tree hacks. Every smart phone shows their company's logo (some kind of > flash) while the phone boots. How do they get around such issues? As far as the memory being used goes, they probably locate the frame buffer well away from the kernel or any area that the kernel is likely to use during decompression. It's probably also marked as a reserved memory region in DT to avoid the kernel touching it during boot, or _maybe_ they just locate it somewhere in memory that they've tested that the kernel doesn't touch until after their kernel has initialised the LCD controller (thereby avoiding the memory being permanently consumed.) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
On 06/29, Russell King - ARM Linux wrote: > On Thu, Jun 29, 2017 at 08:28:08PM +0530, Viresh Kumar wrote: > > On 29-06-17, 13:49, Russell King - ARM Linux wrote: > > > The thing that concerns me most about this is that typically the LCD > > > controller will be performing DMA to system RAM. > > > > > > The location of the frame buffer is unknown to the decompressor - and > > > as the decompressor self-relocates itself (using purely assembly code), > > > it could relocate itself on top of the frame buffer, causing the "nice" > > > image to become very colourful. > > > > > > The decompressor doesn't have the information from DT at that point to > > > know what are safe locations, so it's up to the boot loader to place > > > the frame buffer somewhere out of the way. (If people want to write > > > a DT parser in position independent ARM assembly code that may change.) > > > > > > As long as people realise this, then it's not a problem, but given the > > > number of problems that we've already encountered with boot loaders and > > > memory space layout, I don't trust them to get this right. > > > > > > Right now, the ARM kernel booting document requires: > > > > > > - Quiesce all DMA capable devices so that memory does not get > > > corrupted by bogus network packets or disk data. This will save > > > you many hours of debug. > > > > > > so we would need to modify that to make an exception for LCD controllers. > > > However, we definitely can't have devices left enabled which are capable > > > of writing to system memory, or which changing system memory is likely > > > to cause bad effects (eg, packet ring buffers, USB buffers etc, which is > > > really what the above requirement is about.) > > > > Well, LCD was just an example here. But yeah, it is one of the most > > probable case we have. > > > > So, this thing is already working for sure, of course with some out of > > tree hacks. Every smart phone shows their company's logo (some kind of > > flash) while the phone boots. How do they get around such issues? > > As far as the memory being used goes, they probably locate the frame > buffer well away from the kernel or any area that the kernel is likely > to use during decompression. > > It's probably also marked as a reserved memory region in DT to avoid > the kernel touching it during boot, or _maybe_ they just locate it > somewhere in memory that they've tested that the kernel doesn't touch > until after their kernel has initialised the LCD controller (thereby > avoiding the memory being permanently consumed.) > Yes. The display controller is typically pointed to a memory carveout that we treat as reserved in the kernel. I'm fairly certain that we avoid the "permanently consumed" problem by making it a carveout for the display controller, so that when the display controller probes it can take ownership of the memory from the bootloader. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
On 29-06-17, 15:06, Enrico Weigelt, metux IT consult wrote: > On 29.06.2017 14:47, Viresh Kumar wrote: > > >No. Drivers are registered to the kernel (randomly, though we can know > >their order) and devices are registered separately (platform/amba > >devices get registered automatically with DT, hint: > >drivers/of/platform.c). The device core checks while registering > >devices/drivers if their drivers/devices are available or not. If > >yes, then the devices are probed using the drivers. Now the drivers > >must make sure all the dependencies are met at this point, else they > >can return -EPROBE_DEFER and the kernel will try probing them again. > > Could we somehow introduce an strict ordering ? The problem I am trying to solve isn't really related to ordering. Consider this for example: A supply shared between LCD and I2C controller (Not sure if such configurations are there in any of the hardware we have), where the same I2C controller is used to access the LCD controller's registers. Both are enabled at boot and the supply is configured to satisfy both. If the voltage requirements of the I2C controller are below that of LCD, then we can't decide on which one to probe first. We can't probe LCD first as its bus isn't active yt and if we probe I2C first, then it may take the supply down to a level that isn't acceptable for the LCD (which was on from boot). > Maybe by letting the device core know of the dependencies, before > individual probe()'s explicitly ask for them ? That's what we are sorting out in probe() and I am not sure if we need any more intelligence on that. Though, you may want to look at the "functional dependency" stuff, which can be of some help in such cases. Its mentioned in cover-letter as well. > >This should happen in probe, otherwise we are screwed. > > Yes, but the probe result may be deferred, so it's tried again in the > next round. Correct ? Right. > >But the kernel doesn't know how it is configured, there can be so many > >configurable parameters. The kernel needs to do it again by itself. > > Could it read back the config ? First, it may not always be possible to do that. And even if the kernel reads it all well, then it wouldn't know why things are configured the way they are. And trying to read the config in drivers is going to be so so hacky, that we wouldn't want to do it anyway. We need a clean way of doing this, so that the kernel knows of what's going on and that's what this series is targeting here. > By the way: I've got a similar problem w/ gpmc right now: uboot already > sets it up, but the kernel only knows about one CS (for the nand) and > screwes up the others (eg. fpga), so it cant access the fpga . Until > I've sorted out all the parameters for DT (unfortunately, only have the > raw register values), I'll have to rely on an userland test program > to set it all up ... > > >Let me try with an example. A regulator is shared between LCD and DMA > >controller. > > > >Operable ranges of the regulator: 1.8 - 3.0 V > >Range required by LCD: 2.0 - 3.0 V > >Range required by DMA: 1.8 - 2.5 V > > Would a config readback help here ? > > The regulator core then should know that we're already in proper > range for DMA and no need to touch the regulator. No body is going to allow that kind of hacky code to get merged :) -- viresh
On Fri, Jun 30, 2017 at 11:16 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 29-06-17, 15:06, Enrico Weigelt, metux IT consult wrote: >> On 29.06.2017 14:47, Viresh Kumar wrote: >> >> >No. Drivers are registered to the kernel (randomly, though we can know >> >their order) and devices are registered separately (platform/amba >> >devices get registered automatically with DT, hint: >> >drivers/of/platform.c). The device core checks while registering >> >devices/drivers if their drivers/devices are available or not. If >> >yes, then the devices are probed using the drivers. Now the drivers >> >must make sure all the dependencies are met at this point, else they >> >can return -EPROBE_DEFER and the kernel will try probing them again. >> >> Could we somehow introduce an strict ordering ? > > The problem I am trying to solve isn't really related to ordering. > > Consider this for example: > > A supply shared between LCD and I2C controller (Not sure if such > configurations are there in any of the hardware we have), where the > same I2C controller is used to access the LCD controller's registers. > Both are enabled at boot and the supply is configured to satisfy both. > If the voltage requirements of the I2C controller are below that of > LCD, then we can't decide on which one to probe first. We can't probe > LCD first as its bus isn't active yt and if we probe I2C first, then > it may take the supply down to a level that isn't acceptable for the > LCD (which was on from boot). AFAIK regulator constraints are supposed to satisfy all users of it. >> Maybe by letting the device core know of the dependencies, before >> individual probe()'s explicitly ask for them ? > > That's what we are sorting out in probe() and I am not sure if we need > any more intelligence on that. Though, you may want to look at the > "functional dependency" stuff, which can be of some help in such > cases. Its mentioned in cover-letter as well. > >> >This should happen in probe, otherwise we are screwed. >> >> Yes, but the probe result may be deferred, so it's tried again in the >> next round. Correct ? > > Right. > >> >But the kernel doesn't know how it is configured, there can be so many >> >configurable parameters. The kernel needs to do it again by itself. >> >> Could it read back the config ? > > First, it may not always be possible to do that. And even if the > kernel reads it all well, then it wouldn't know why things are > configured the way they are. And trying to read the config in drivers > is going to be so so hacky, that we wouldn't want to do it anyway. We > need a clean way of doing this, so that the kernel knows of what's > going on and that's what this series is targeting here. > >> By the way: I've got a similar problem w/ gpmc right now: uboot already >> sets it up, but the kernel only knows about one CS (for the nand) and >> screwes up the others (eg. fpga), so it cant access the fpga . Until >> I've sorted out all the parameters for DT (unfortunately, only have the >> raw register values), I'll have to rely on an userland test program >> to set it all up ... >> >> >Let me try with an example. A regulator is shared between LCD and DMA >> >controller. >> > >> >Operable ranges of the regulator: 1.8 - 3.0 V >> >Range required by LCD: 2.0 - 3.0 V >> >Range required by DMA: 1.8 - 2.5 V So for the example here, the regulator constraint should be 2.5 - 3.0 V, or the intersection of all voltage requirements. ChenYu >> >> Would a config readback help here ? >> >> The regulator core then should know that we're already in proper >> range for DMA and no need to touch the regulator. > > No body is going to allow that kind of hacky code to get merged :) > > -- > viresh > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 30-06-17, 11:33, Chen-Yu Tsai wrote: > AFAIK regulator constraints are supposed to satisfy all users of it. Right. > >> >Let me try with an example. A regulator is shared between LCD and DMA > >> >controller. > >> > > >> >Operable ranges of the regulator: 1.8 - 3.0 V > >> >Range required by LCD: 2.0 - 3.0 V > >> >Range required by DMA: 1.8 - 2.5 V > > So for the example here, the regulator constraint should be 2.5 - 3.0 V, > or the intersection of all voltage requirements. Had a look at regulator_check_consumers() and the range selected by it is the *highest* min_uV and *lowest* max_uV, to find that intersection point. For LCD: min_uV = 2.0 V, max_uV = 3.0 V For DMA: min_uV = 1.8 V, max_uV = 2.5 V Highest min_uV = 2.0 V Lowest max_uV = 2.5 V And so I mentioned the regulator's final range (that satisfies all consumers) is 2 - 2.5 V. Why do you say it should be 2.5 - 3.0 V ? -- viresh
On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 30-06-17, 11:33, Chen-Yu Tsai wrote: >> AFAIK regulator constraints are supposed to satisfy all users of it. > > Right. > >> >> >Let me try with an example. A regulator is shared between LCD and DMA >> >> >controller. >> >> > >> >> >Operable ranges of the regulator: 1.8 - 3.0 V >> >> >Range required by LCD: 2.0 - 3.0 V >> >> >Range required by DMA: 1.8 - 2.5 V >> >> So for the example here, the regulator constraint should be 2.5 - 3.0 V, >> or the intersection of all voltage requirements. > > Had a look at regulator_check_consumers() and the range selected by it > is the *highest* min_uV and *lowest* max_uV, to find that intersection > point. > > For LCD: min_uV = 2.0 V, max_uV = 3.0 V > For DMA: min_uV = 1.8 V, max_uV = 2.5 V > > Highest min_uV = 2.0 V > Lowest max_uV = 2.5 V > > And so I mentioned the regulator's final range (that satisfies all > consumers) is 2 - 2.5 V. > > Why do you say it should be 2.5 - 3.0 V ? You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this morning. :( I also want to mention that for DT based platforms, this constraint should already be set in the device tree for the regulator, so the scenario where DMA comes up and sets a voltage level that LCD cannot use should not even be possible. ChenYu
On 30-06-17, 12:05, Chen-Yu Tsai wrote: > On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 30-06-17, 11:33, Chen-Yu Tsai wrote: > >> AFAIK regulator constraints are supposed to satisfy all users of it. > > > > Right. > > > >> >> >Let me try with an example. A regulator is shared between LCD and DMA > >> >> >controller. > >> >> > > >> >> >Operable ranges of the regulator: 1.8 - 3.0 V > >> >> >Range required by LCD: 2.0 - 3.0 V > >> >> >Range required by DMA: 1.8 - 2.5 V > >> > >> So for the example here, the regulator constraint should be 2.5 - 3.0 V, > >> or the intersection of all voltage requirements. > > > > Had a look at regulator_check_consumers() and the range selected by it > > is the *highest* min_uV and *lowest* max_uV, to find that intersection > > point. > > > > For LCD: min_uV = 2.0 V, max_uV = 3.0 V > > For DMA: min_uV = 1.8 V, max_uV = 2.5 V > > > > Highest min_uV = 2.0 V > > Lowest max_uV = 2.5 V > > > > And so I mentioned the regulator's final range (that satisfies all > > consumers) is 2 - 2.5 V. > > > > Why do you say it should be 2.5 - 3.0 V ? > > You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this > morning. :( And I was worrying if I had something else in my coffee :) > I also want to mention that for DT based platforms, this constraint > should already be set in the device tree for the regulator, so the > scenario where DMA comes up and sets a voltage level that LCD cannot > use should not even be possible. Yes, such constraints are already present. But the problem (this series is trying to solve) is that the kernel doesn't know if the LCD is already powered ON. And so when DMA gets probed first, the kernel thinks that DMA is the only user of the regulator and the voltage is set to 1.8-2.5 V. And so this series is somehow trying to make the kernel aware about the constraints of the LCD controller which was enabled in the bootloader. -- viresh
On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 30-06-17, 12:05, Chen-Yu Tsai wrote: >> On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> > On 30-06-17, 11:33, Chen-Yu Tsai wrote: >> >> AFAIK regulator constraints are supposed to satisfy all users of it. >> > >> > Right. >> > >> >> >> >Let me try with an example. A regulator is shared between LCD and DMA >> >> >> >controller. >> >> >> > >> >> >> >Operable ranges of the regulator: 1.8 - 3.0 V >> >> >> >Range required by LCD: 2.0 - 3.0 V >> >> >> >Range required by DMA: 1.8 - 2.5 V >> >> >> >> So for the example here, the regulator constraint should be 2.5 - 3.0 V, >> >> or the intersection of all voltage requirements. >> > >> > Had a look at regulator_check_consumers() and the range selected by it >> > is the *highest* min_uV and *lowest* max_uV, to find that intersection >> > point. >> > >> > For LCD: min_uV = 2.0 V, max_uV = 3.0 V >> > For DMA: min_uV = 1.8 V, max_uV = 2.5 V >> > >> > Highest min_uV = 2.0 V >> > Lowest max_uV = 2.5 V >> > >> > And so I mentioned the regulator's final range (that satisfies all >> > consumers) is 2 - 2.5 V. >> > >> > Why do you say it should be 2.5 - 3.0 V ? >> >> You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this >> morning. :( > > And I was worrying if I had something else in my coffee :) > >> I also want to mention that for DT based platforms, this constraint >> should already be set in the device tree for the regulator, so the >> scenario where DMA comes up and sets a voltage level that LCD cannot >> use should not even be possible. > > Yes, such constraints are already present. But the problem (this > series is trying to solve) is that the kernel doesn't know if the LCD > is already powered ON. And so when DMA gets probed first, the kernel > thinks that DMA is the only user of the regulator and the voltage is > set to 1.8-2.5 V. And so this series is somehow trying to make the > kernel aware about the constraints of the LCD controller which was > enabled in the bootloader. What I'm saying is for the DT case, the constraints are already limited to the intersection of all users, regardless of whether they are turned on or not. At least this is what I believe makes sense. You really don't want to set a regulator such that it over voltages for a subset of its consumers. Consumers might not have proper power isolation for this. I think what you mean is that the DT constraints are the union of all consumer constraints (1.8 - 3.0 V in this case), then each consumer comes in and adds its own constraints. And for such a design, the kernel needs to know which and what constraints to apply. Either way regulators already support constraints, so they are easier to deal with. Clocks on the other hand, while the core does support clock rate constraints, AFAIK no one really uses or supports them. ChenYu
On 30-06-17, 12:22, Chen-Yu Tsai wrote: > On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 30-06-17, 12:05, Chen-Yu Tsai wrote: > >> I also want to mention that for DT based platforms, this constraint > >> should already be set in the device tree for the regulator, so the > >> scenario where DMA comes up and sets a voltage level that LCD cannot > >> use should not even be possible. > > What I'm saying is for the DT case, the constraints are already limited > to the intersection of all users, regardless of whether they are turned > on or not. Right, but someone needs to get the regulator first to have that considered by the regulator core while deciding the final range. Both DMA and LCD driver do regulator_get() for their devices but if only DMA driver is probed until now, then the regulator core wouldn't consider LCD as regulator_get() is never called for LCD. > I think what you mean is that the DT constraints are the union of all > consumer constraints (1.8 - 3.0 V in this case), then each consumer > comes in and adds its own constraints. And for such a design, the kernel > needs to know which and what constraints to apply. Sorry, I am confused with what you just said and not sure if I understand it completely. Each consumer DT node will have its own set of constraints for the regulator device. The kernel will do regulator_get() for them one by one, based on when their drivers get probed. And an intersection of those constraints (which already did regulator_get()) will be used by the regulator core. Now this series is saying that even if the driver didn't come up (for LCD) and haven't done its regulator_get() yet, consider that device's constraint while calculating the target voltage for the regulator. > Either way regulators already support constraints, so they are easier > to deal with. Clocks on the other hand, while the core does support > clock rate constraints, AFAIK no one really uses or supports them. Yeah, so I started with just regulators and that's when Mark suggested to do something generic which can be reused by other resource types. We may end up covering clk for sure I believe. Not sure yet about other resource types though. -- viresh
= On Fri, Jun 30, 2017 at 1:12 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 30-06-17, 12:22, Chen-Yu Tsai wrote: >> On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> > On 30-06-17, 12:05, Chen-Yu Tsai wrote: > >> >> I also want to mention that for DT based platforms, this constraint >> >> should already be set in the device tree for the regulator, so the >> >> scenario where DMA comes up and sets a voltage level that LCD cannot >> >> use should not even be possible. >> >> What I'm saying is for the DT case, the constraints are already limited >> to the intersection of all users, regardless of whether they are turned >> on or not. > > Right, but someone needs to get the regulator first to have that > considered by the regulator core while deciding the final range. AFAIK the regulator core automatically corrects any voltages outside its constraints when the regulator is first registered. This is independent of any consumer constraints. > Both DMA and LCD driver do regulator_get() for their devices but if > only DMA driver is probed until now, then the regulator core wouldn't > consider LCD as regulator_get() is never called for LCD. > >> I think what you mean is that the DT constraints are the union of all >> consumer constraints (1.8 - 3.0 V in this case), then each consumer >> comes in and adds its own constraints. And for such a design, the kernel >> needs to know which and what constraints to apply. > > Sorry, I am confused with what you just said and not sure if I > understand it completely. > > Each consumer DT node will have its own set of constraints for the > regulator device. The kernel will do regulator_get() for them one by > one, based on when their drivers get probed. And an intersection of > those constraints (which already did regulator_get()) will be used by > the regulator core. No. In the device tree, the only constraints (per the current state of the bindings) is for the regulator supply. Any consumer constraints are programmed purely by the driver, by using regulator_set_voltage(). All of them are considered by the core before setting the real voltage. > Now this series is saying that even if the driver didn't come up (for > LCD) and haven't done its regulator_get() yet, consider that device's > constraint while calculating the target voltage for the regulator. What I'm saying is that, for the constraints in the regulator supply node, you would have already considered all consumer constraints. If one of its consumers can't take power above 2.5 V, surely you don't want the regulator sending power above that, so you would have regulator-max-microvolt = <2500000>; for that regulator node. You would do something similar for the lower limit of the voltage range. You don't even need any actual consumers. Using regulator-min-microvolt, regulator-max-microvolt, and regulator-always-on, you can have a regulator provide power at a suitable voltage. We do this for various power rails on Allwinner SoCs that don't really have proper consumers, like power for internal logic, PLL, and others. I'm not saying this is a good solution, because you lose runtime control of the regulator. It's just something we came up with in lieu of any proper consumers. >> Either way regulators already support constraints, so they are easier >> to deal with. Clocks on the other hand, while the core does support >> clock rate constraints, AFAIK no one really uses or supports them. > > Yeah, so I started with just regulators and that's when Mark suggested > to do something generic which can be reused by other resource types. > We may end up covering clk for sure I believe. Not sure yet about > other resource types though. This might be unrelated, but I think it is a similar problem. When a clk rate change is propagated up the clk tree, any affected sibling clks aren't automatically readjusted, i.e. try to keep roughly the same output clk rate by adjusting its own dividers. This might be one side of the problem you are trying to solve. ChenYu
On 30-06-17, 14:36, Chen-Yu Tsai wrote: > = On Fri, Jun 30, 2017 at 1:12 PM, Viresh Kumar > <viresh.kumar@linaro.org> wrote: > > Right, but someone needs to get the regulator first to have that > > considered by the regulator core while deciding the final range. > > AFAIK the regulator core automatically corrects any voltages outside > its constraints when the regulator is first registered. This is > independent of any consumer constraints. Right, so the kernel checks if the current voltage value set for the supply is within valid range as per the constraints present in regulator node. > > Both DMA and LCD driver do regulator_get() for their devices but if > > only DMA driver is probed until now, then the regulator core wouldn't > > consider LCD as regulator_get() is never called for LCD. > > > >> I think what you mean is that the DT constraints are the union of all > >> consumer constraints (1.8 - 3.0 V in this case), then each consumer > >> comes in and adds its own constraints. And for such a design, the kernel > >> needs to know which and what constraints to apply. > > > > Sorry, I am confused with what you just said and not sure if I > > understand it completely. > > > > Each consumer DT node will have its own set of constraints for the > > regulator device. The kernel will do regulator_get() for them one by > > one, based on when their drivers get probed. And an intersection of > > those constraints (which already did regulator_get()) will be used by > > the regulator core. > > No. In the device tree, the only constraints (per the current state > of the bindings) is for the regulator supply. Any consumer constraints > are programmed purely by the driver, by using regulator_set_voltage(). > All of them are considered by the core before setting the real voltage. That's right. I had a bit of misunderstanding here. By default min/max for the consumers is set to zero and they are ignored in regulator_check_consumers(). > > Now this series is saying that even if the driver didn't come up (for > > LCD) and haven't done its regulator_get() yet, consider that device's > > constraint while calculating the target voltage for the regulator. > > What I'm saying is that, for the constraints in the regulator supply node, > you would have already considered all consumer constraints. Yes, so whatever voltage the bootloader has programmed for LCD should fit within constraints present in supply's node. > If one of its > consumers can't take power above 2.5 V, surely you don't want the regulator > sending power above that, so you would have > > regulator-max-microvolt = <2500000>; > > for that regulator node. You would do something similar for the lower > limit of the voltage range. > I am no regulators expert, but AFAIU that's not true. What's wrong with the following scenario: Operable ranges of the regulator: 1.8 - 3.0 V Range required by LCD: 2.0 - 3.0 V Range required by DMA: 1.8 - 2.5 V Here DMA can't work with regulator voltages > 2.5 V, but regulator can go max to 3.0 V. Of course if the DMA driver has done regulator_set_voltage(), then we will be within 2.5 V range. But that doesn't force us to have regulator-max-microvolt set to 2.5 V. And so the DT node shall have this: regulator-min-microvolt = <1800000>; regulator-max-microvolt = <3000000>; Isn't it ? > This might be unrelated, but I think it is a similar problem. When a > clk rate change is propagated up the clk tree, any affected sibling > clks aren't automatically readjusted, i.e. try to keep roughly the > same output clk rate by adjusting its own dividers. This might be > one side of the problem you are trying to solve. I am not sure how this problem can be solved with what this set is proposing. :( -- viresh
On Fri, Jun 30, 2017 at 02:13:30PM +0530, Viresh Kumar wrote: > On 30-06-17, 14:36, Chen-Yu Tsai wrote: > Operable ranges of the regulator: 1.8 - 3.0 V > Range required by LCD: 2.0 - 3.0 V > Range required by DMA: 1.8 - 2.5 V > Here DMA can't work with regulator voltages > 2.5 V, but regulator can > go max to 3.0 V. Of course if the DMA driver has done > regulator_set_voltage(), then we will be within 2.5 V range. But that > doesn't force us to have regulator-max-microvolt set to 2.5 V. > And so the DT node shall have this: > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <3000000>; > Isn't it ? If the DMA can't tolerate more than 2.5V then why would the constraints allow the voltage to float that far? Similarly on the low end? Please remember that devices shouldn't be managing their voltages unless they are actively changing them at runtime, simply setting them at startup is the job of the constraints. I would be very surprised to see a DMA controller doing anything like DVFS. > > > This might be unrelated, but I think it is a similar problem. When a > > clk rate change is propagated up the clk tree, any affected sibling > > clks aren't automatically readjusted, i.e. try to keep roughly the > > same output clk rate by adjusting its own dividers. This might be > > one side of the problem you are trying to solve. > > I am not sure how this problem can be solved with what this set is > proposing. :( > > -- > viresh
On Fri, Jun 30, 2017 at 12:22:13PM +0800, Chen-Yu Tsai wrote: > What I'm saying is for the DT case, the constraints are already limited > to the intersection of all users, regardless of whether they are turned > on or not. At least this is what I believe makes sense. You really don't > want to set a regulator such that it over voltages for a subset of its > consumers. Consumers might not have proper power isolation for this. Right, and we also shouldn't have voltage setting code in consumers that don't need to vary their voltage at runtime. This keeps complexity out of drivers and avoids the need to handle things like variants that have different power requirements. > I think what you mean is that the DT constraints are the union of all > consumer constraints (1.8 - 3.0 V in this case), then each consumer > comes in and adds its own constraints. And for such a design, the kernel > needs to know which and what constraints to apply. That's the broad idea.
On 30-06-17, 13:10, Mark Brown wrote: > On Fri, Jun 30, 2017 at 02:13:30PM +0530, Viresh Kumar wrote: > > On 30-06-17, 14:36, Chen-Yu Tsai wrote: > > > Operable ranges of the regulator: 1.8 - 3.0 V > > Range required by LCD: 2.0 - 3.0 V > > Range required by DMA: 1.8 - 2.5 V > > > Here DMA can't work with regulator voltages > 2.5 V, but regulator can > > go max to 3.0 V. Of course if the DMA driver has done > > regulator_set_voltage(), then we will be within 2.5 V range. But that > > doesn't force us to have regulator-max-microvolt set to 2.5 V. > > > And so the DT node shall have this: > > > regulator-min-microvolt = <1800000>; > > regulator-max-microvolt = <3000000>; > > > Isn't it ? > > If the DMA can't tolerate more than 2.5V then why would the constraints > allow the voltage to float that far? Similarly on the low end? The above regulator-min/max-microvolt values I mentioned were for the regulator device and not what the consumers would request. Yes, DMA will request something between 1.8 to 2.5 V, but in the above example LCD can request from 2.0 to 3.0 V and so I had those limits for the regulator device (in DT). > Please remember that devices shouldn't be managing their voltages unless > they are actively changing them at runtime, simply setting them at > startup is the job of the constraints. I would be very surprised to see > a DMA controller doing anything like DVFS. Sure, DMA would most likely set a constraint from probe. Maybe I could have used MMC in the above example, which may actually do DVFS at runtime. -- viresh
On Mon, Jul 03, 2017 at 11:45:52AM +0530, Viresh Kumar wrote: > On 30-06-17, 13:10, Mark Brown wrote: > > On Fri, Jun 30, 2017 at 02:13:30PM +0530, Viresh Kumar wrote: > > > On 30-06-17, 14:36, Chen-Yu Tsai wrote: > > > And so the DT node shall have this: > > > regulator-min-microvolt = <1800000>; > > > regulator-max-microvolt = <3000000>; > > > Isn't it ? > > If the DMA can't tolerate more than 2.5V then why would the constraints > > allow the voltage to float that far? Similarly on the low end? > The above regulator-min/max-microvolt values I mentioned were for the regulator > device and not what the consumers would request. Yes, DMA will request something If you're putting the maximum possible range that the physical regulator can supply into machine constraints then you really haven't understood what machine constraints are at all. > > Please remember that devices shouldn't be managing their voltages unless > > they are actively changing them at runtime, simply setting them at > > startup is the job of the constraints. I would be very surprised to see > > a DMA controller doing anything like DVFS. > Sure, DMA would most likely set a constraint from probe. Maybe I could have used No, it really shouldn't. Please read what I wrote. > MMC in the above example, which may actually do DVFS at runtime. Yes, MMC does vary voltage.
On 03-07-17, 16:07, Mark Brown wrote: > On Mon, Jul 03, 2017 at 11:45:52AM +0530, Viresh Kumar wrote: > > The above regulator-min/max-microvolt values I mentioned were for the regulator > > device and not what the consumers would request. Yes, DMA will request something > > If you're putting the maximum possible range that the physical regulator > can supply into machine constraints then you really haven't understood > what machine constraints are at all. I wasn't referring to the limits of the physical regulators but the min/max that the consumers can set on a particular platform. > No, it really shouldn't. Please read what I wrote. Sorry about that. Understood it now. -- viresh
On Thu, Jun 29, 2017 at 5:00 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 06/29, Russell King - ARM Linux wrote: >> On Thu, Jun 29, 2017 at 08:28:08PM +0530, Viresh Kumar wrote: >> > On 29-06-17, 13:49, Russell King - ARM Linux wrote: >> > > The thing that concerns me most about this is that typically the LCD >> > > controller will be performing DMA to system RAM. >> > > >> > > The location of the frame buffer is unknown to the decompressor - and >> > > as the decompressor self-relocates itself (using purely assembly code), >> > > it could relocate itself on top of the frame buffer, causing the "nice" >> > > image to become very colourful. >> > > >> > > The decompressor doesn't have the information from DT at that point to >> > > know what are safe locations, so it's up to the boot loader to place >> > > the frame buffer somewhere out of the way. (If people want to write >> > > a DT parser in position independent ARM assembly code that may change.) >> > > >> > > As long as people realise this, then it's not a problem, but given the >> > > number of problems that we've already encountered with boot loaders and >> > > memory space layout, I don't trust them to get this right. >> > > >> > > Right now, the ARM kernel booting document requires: >> > > >> > > - Quiesce all DMA capable devices so that memory does not get >> > > corrupted by bogus network packets or disk data. This will save >> > > you many hours of debug. >> > > >> > > so we would need to modify that to make an exception for LCD controllers. >> > > However, we definitely can't have devices left enabled which are capable >> > > of writing to system memory, or which changing system memory is likely >> > > to cause bad effects (eg, packet ring buffers, USB buffers etc, which is >> > > really what the above requirement is about.) >> > >> > Well, LCD was just an example here. But yeah, it is one of the most >> > probable case we have. >> > >> > So, this thing is already working for sure, of course with some out of >> > tree hacks. Every smart phone shows their company's logo (some kind of >> > flash) while the phone boots. How do they get around such issues? >> >> As far as the memory being used goes, they probably locate the frame >> buffer well away from the kernel or any area that the kernel is likely >> to use during decompression. >> >> It's probably also marked as a reserved memory region in DT to avoid >> the kernel touching it during boot, or _maybe_ they just locate it >> somewhere in memory that they've tested that the kernel doesn't touch >> until after their kernel has initialised the LCD controller (thereby >> avoiding the memory being permanently consumed.) >> > > Yes. The display controller is typically pointed to a memory > carveout that we treat as reserved in the kernel. I'm fairly > certain that we avoid the "permanently consumed" problem by > making it a carveout for the display controller, so that when the > display controller probes it can take ownership of the memory > from the bootloader. > For aarch64/arm64 booting with EFI, the bootloader passes info about memory layout to the kernel (including the fact that the framebuffer is carved out) so kernel doesn't clobber the scanout buffer. The non-EFI case, the bootloader should (not that lk does) patch up the fdt reserved memory node w/ correct address/size. I think lk+downstream just relies on luck. (I have a patch that makes lk insert a framebuffer-simple node, which I think should also solve it in the non-bootefi case) BR, -R
On 07/05, Rob Clark wrote: > On Thu, Jun 29, 2017 at 5:00 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > > On 06/29, Russell King - ARM Linux wrote: > >> > >> As far as the memory being used goes, they probably locate the frame > >> buffer well away from the kernel or any area that the kernel is likely > >> to use during decompression. > >> > >> It's probably also marked as a reserved memory region in DT to avoid > >> the kernel touching it during boot, or _maybe_ they just locate it > >> somewhere in memory that they've tested that the kernel doesn't touch > >> until after their kernel has initialised the LCD controller (thereby > >> avoiding the memory being permanently consumed.) > >> > > > > Yes. The display controller is typically pointed to a memory > > carveout that we treat as reserved in the kernel. I'm fairly > > certain that we avoid the "permanently consumed" problem by > > making it a carveout for the display controller, so that when the > > display controller probes it can take ownership of the memory > > from the bootloader. > > > > For aarch64/arm64 booting with EFI, the bootloader passes info about > memory layout to the kernel (including the fact that the framebuffer > is carved out) so kernel doesn't clobber the scanout buffer. The > non-EFI case, the bootloader should (not that lk does) patch up the > fdt reserved memory node w/ correct address/size. I think > lk+downstream just relies on luck. > Downstream+lk seems to be doing a carveout with reserved-memory in DT[1]. The bootloader isn't updating the DT to indicate this, instead we rely on an agreement between bootloader and kernel's DT file to have the carveout. Once the display driver configures things enough to allocate another display buffer it actually frees the memory back to the system[2]. Most of the code for this splash screen stuff is in that mdss_mdp_splash_logo.c file. [1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8996.dtsi?h=rel/msm-4.4#n231 [2] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/drivers/video/fbdev/msm/mdss_mdp_splash_logo.c?h=rel/msm-4.4#n252 -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Hi, I am sending this RFC to get early comments before I put too much effort in the solution proposed here. The solution isn't fully complete yet. Problem statement: Some devices are powered ON by the bootloaders before the bootloader handovers control to Linux. It maybe important for those devices to keep working until the time a Linux device driver probes the device and reconfigure its resources. A typical example of that can be the LCD controller, which is used by the bootloaders to show image(s) while the machine is booting into Linux. The LCD controller can be using some resources, like clk, regulators, etc, that are shared between several devices. These shared resources should be programmed so that all the users of them are satisfied. If a user (X) driver gets probed before the LCD controller driver in this case, then it may end up reconfiguring these resources to ranges satisfying the current users (only user X) and that can make the LCD screen unstable. Of course we can have more complex cases where the same resource is getting used by two devices while the kernel boots and the order in which devices get probed wouldn't matter as the other device will surely break then. Proposed solution: This patchset introduces the concept of boot-constraints, which are set by the different parts of the kernel (on behalf of the bootloaders) and the kernel will satisfy them until the time driver for such a device is probed (successfully or unsuccessfully). Once the driver's probe() routine is called, the driver core removes the constraints set for the particular device. Only the power-supply constraint type is supported for now. This can be used across platforms working with DT, ACPI, etc and has no dependency on those. What's left ? There are a couple of problems which aren't solved yet: o EPROBE_DEFER: Deferred probing is a major problem that needs to be solved. Because we want to add the constraint ASAP (i.e. before most of the drivers are registered), we would end up doing that right after the device is created. That is very early in the kernel boot cycle and its very much possible that the resource we need to get (in order to set the constraint) isn't available yet. Like regulator in case of supply constraint. Now how do we control that the constraint is set right after the resource is available, and before any other user of the resource comes up ? One way out is to set "driver_deferred_probe_enable" to 'true' (drivers/base/dd.c), after we got EPROBE_DEFER for any of the constraints. That would make sure that all the deferred drivers get a chance to get probed again, after addition of every new device. Without this change, we probe all the deferred devices only after late_init. But that may not work as it depends on a new workqueue thread for doing this work, and no one is stopping another driver to get registered by that time. I also thought about using the "functional dependency" stuff [1] that Rafael introduced earlier, but that too has its own challenges. Specifically, we may not have the device structures available for all the consumers/suppliers to start with. o DT support will be added at first and I am planning to add the constraints right from drivers/of/platform.c after the AMBA and platform devices are created automatically from DT. Some sort of bindings (per constraint type) are required to be defined for that of course. We can think about other interfaces (like ACPI) as well, if we have any users right now. But yeah, the first thing is to get some sort of feedback for the proposed solution. :) FWIW, I started discussing this with Mark Brown earlier [2] and this series is a follow-up to that discussion. -- viresh [1] commit 9ed9895370ae ("driver core: Functional dependencies tracking support") [2] https://marc.info/?l=linux-kernel&m=149423887617635 Viresh Kumar (5): drivers: Add boot constraints core drivers: boot_constraint: Add support for supply constraints drivers: boot_constraint: Add boot_constraints_disable kernel parameter drivers: boot_constraint: Add debugfs support drivers: Code to test boot constraints Documentation/admin-guide/kernel-parameters.txt | 2 + drivers/base/Kconfig | 11 + drivers/base/Makefile | 1 + drivers/base/boot_constraint.c | 402 ++++++++++++++++++++++++ drivers/base/dd.c | 20 +- drivers/base/test_plat_boot_constraint.c | 51 +++ include/linux/boot_constraint.h | 35 +++ 7 files changed, 515 insertions(+), 7 deletions(-) create mode 100644 drivers/base/boot_constraint.c create mode 100644 drivers/base/test_plat_boot_constraint.c create mode 100644 include/linux/boot_constraint.h -- 2.13.0.71.gd7076ec9c9cb