[alsa-devel] [PATCH] asoc/multi-component: fsl: add support for variable SSI FIFO depth
Add code that programs the DMA and SSI controllers differently based on the FIFO depth of the SSI.
The SSI devices on the MPC8610 and the P1022 are identical in every way except one: the transmit and receive FIFO depth. On the MPC8610, the depth is eight. On the P1022, it's fifteen. The device tree nodes for the SSI include a "fsl,fifo-depth" property that specifies the FIFO depth.
Signed-off-by: Timur Tabi timur@freescale.com --- sound/soc/fsl/fsl_dma.c | 69 ++++++++++++++++++++++++++++++++++++----------- sound/soc/fsl/fsl_ssi.c | 27 ++++++++++++++++-- 2 files changed, 77 insertions(+), 19 deletions(-)
diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c index 57774cb..2f1461a 100644 --- a/sound/soc/fsl/fsl_dma.c +++ b/sound/soc/fsl/fsl_dma.c @@ -60,6 +60,7 @@ struct dma_object { struct snd_soc_platform_driver dai; dma_addr_t ssi_stx_phys; dma_addr_t ssi_srx_phys; + unsigned int ssi_fifo_depth; struct ccsr_dma_channel __iomem *channel; unsigned int irq; bool assigned; @@ -99,6 +100,7 @@ struct fsl_dma_private { unsigned int irq; struct snd_pcm_substream *substream; dma_addr_t ssi_sxx_phys; + unsigned int ssi_fifo_depth; dma_addr_t ld_buf_phys; unsigned int current_link; dma_addr_t dma_buf_phys; @@ -431,6 +433,7 @@ static int fsl_dma_open(struct snd_pcm_substream *substream) else dma_private->ssi_sxx_phys = dma->ssi_srx_phys;
+ dma_private->ssi_fifo_depth = dma->ssi_fifo_depth; dma_private->dma_channel = dma->channel; dma_private->irq = dma->irq; dma_private->substream = substream; @@ -544,11 +547,11 @@ static int fsl_dma_hw_params(struct snd_pcm_substream *substream, struct device *dev = rtd->platform->dev;
/* Number of bits per sample */ - unsigned int sample_size = + unsigned int sample_bits = snd_pcm_format_physical_width(params_format(hw_params));
/* Number of bytes per frame */ - unsigned int frame_size = 2 * (sample_size / 8); + unsigned int sample_bytes = sample_bits / 8;
/* Bus address of SSI STX register */ dma_addr_t ssi_sxx_phys = dma_private->ssi_sxx_phys; @@ -588,7 +591,7 @@ static int fsl_dma_hw_params(struct snd_pcm_substream *substream, * that offset here. While we're at it, also tell the DMA controller * how much data to transfer per sample. */ - switch (sample_size) { + switch (sample_bits) { case 8: mr |= CCSR_DMA_MR_DAHTS_1 | CCSR_DMA_MR_SAHTS_1; ssi_sxx_phys += 3; @@ -602,22 +605,42 @@ static int fsl_dma_hw_params(struct snd_pcm_substream *substream, break; default: /* We should never get here */ - dev_err(dev, "unsupported sample size %u\n", sample_size); + dev_err(dev, "unsupported sample size %u\n", sample_bits); return -EINVAL; }
/* - * BWC should always be a multiple of the frame size. BWC determines - * how many bytes are sent/received before the DMA controller checks the - * SSI to see if it needs to stop. For playback, the transmit FIFO can - * hold three frames, so we want to send two frames at a time. For - * capture, the receive FIFO is triggered when it contains one frame, so - * we want to receive one frame at a time. + * BWC determines how many bytes are sent/received before the DMA + * controller checks the SSI to see if it needs to stop. BWC should + * always be a multiple of the frame size, so that we always transmit + * whole frames. Each frame occupies two slots in the FIFO. The + * parameter for CCSR_DMA_MR_BWC() is rounded down the next power of two + * (MR[BWC] can only represent even powers of two). + * + * To simplify the process, we set BWC to the largest value that is + * less than or equal to the FIFO watermark. For playback, this ensures + * that we transfer the maximum amount without overrunning the FIFO. + * For capture, this ensures that we transfer the maximum amount without + * underrunning the FIFO. + * + * f = SSI FIFO depth + * w = SSI watermark value (which equals f - 2) + * b = DMA bandwidth count (in bytes) + * s = sample size (in bytes, which equals frame_size * 2) + * + * For playback, we never transmit more than the transmit FIFO + * watermark, otherwise we might write more data than the FIFO can hold. + * The watermark is equal to the FIFO depth minus two. + * + * For capture, two equations must hold: + * w > f - (b / s) + * w >= b / s + * + * So, b > 2 * s, but b must also be <= s * w. To simplify, we set + * b = s * w, which is equal to + * (dma_private->ssi_fifo_depth - 2) * sample_bytes. */ - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - mr |= CCSR_DMA_MR_BWC(2 * frame_size); - else - mr |= CCSR_DMA_MR_BWC(frame_size); + mr |= CCSR_DMA_MR_BWC((dma_private->ssi_fifo_depth - 2) * sample_bytes);
out_be32(&dma_channel->mr, mr);
@@ -871,6 +894,7 @@ static int __devinit fsl_soc_dma_probe(struct of_device *of_dev, struct device_node *np = of_dev->dev.of_node; struct device_node *ssi_np; struct resource res; + const uint32_t *iprop; int ret;
/* Find the SSI node that points to us. */ @@ -881,15 +905,17 @@ static int __devinit fsl_soc_dma_probe(struct of_device *of_dev, }
ret = of_address_to_resource(ssi_np, 0, &res); - of_node_put(ssi_np); if (ret) { - dev_err(&of_dev->dev, "could not determine device resources\n"); + dev_err(&of_dev->dev, "could not determine resources for %s\n", + ssi_np->full_name); + of_node_put(ssi_np); return ret; }
dma = kzalloc(sizeof(*dma) + strlen(np->full_name), GFP_KERNEL); if (!dma) { dev_err(&of_dev->dev, "could not allocate dma object\n"); + of_node_put(ssi_np); return -ENOMEM; }
@@ -902,6 +928,17 @@ static int __devinit fsl_soc_dma_probe(struct of_device *of_dev, dma->ssi_stx_phys = res.start + offsetof(struct ccsr_ssi, stx0); dma->ssi_srx_phys = res.start + offsetof(struct ccsr_ssi, srx0);
+ iprop = of_get_property(ssi_np, "fsl,fifo-depth", NULL); + if (!iprop) { + dev_err(&of_dev->dev, "missing fsl,fifo-depth property in %s\n", + ssi_np->full_name); + kfree(dma); + of_node_put(ssi_np); + return -ENODEV; + } + dma->ssi_fifo_depth = *iprop; + of_node_put(ssi_np); + ret = snd_soc_register_platform(&of_dev->dev, &dma->dai); if (ret) { dev_err(&of_dev->dev, "could not register platform\n"); diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 00e3e62..a0e18b8 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -93,6 +93,7 @@ struct fsl_ssi_private { unsigned int playback; unsigned int capture; int asynchronous; + unsigned int fifo_depth; struct snd_soc_dai_driver cpu_dai_drv; struct device_attribute dev_attr; struct platform_device *pdev; @@ -337,11 +338,20 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
/* * Set the watermark for transmit FIFI 0 and receive FIFO 0. We - * don't use FIFO 1. Since the SSI only supports stereo, the - * watermark should never be an odd number. + * don't use FIFO 1. We program the transmit water to signal a + * DMA transfer if there are only two (or fewer) elements left + * in the FIFO. Two elements equals one frame (left channel, + * right channel). This value, however, depends on the depth of + * the transmit buffer. + * + * We program the receive FIFO to notify us if at least two + * elements (one frame) have been written to the FIFO. We could + * make this value larger (and maybe we should), but this way + * data will be written to memory as soon as it's available. */ out_be32(&ssi->sfcsr, - CCSR_SSI_SFCSR_TFWM0(6) | CCSR_SSI_SFCSR_RFWM0(2)); + CCSR_SSI_SFCSR_TFWM0(ssi_private->fifo_depth - 2) | + CCSR_SSI_SFCSR_RFWM0(ssi_private->fifo_depth - 2));
/* * We keep the SSI disabled because if we enable it, then the @@ -622,6 +632,7 @@ static int __devinit fsl_ssi_probe(struct of_device *of_dev, struct device_attribute *dev_attr = NULL; struct device_node *np = of_dev->dev.of_node; const char *p, *sprop; + const uint32_t *iprop; struct resource res; char name[64];
@@ -671,6 +682,16 @@ static int __devinit fsl_ssi_probe(struct of_device *of_dev, else ssi_private->cpu_dai_drv.symmetric_rates = 1;
+ /* Determine the FIFO depth. */ + iprop = of_get_property(np, "fsl,fifo-depth", NULL); + if (!iprop) { + dev_err(&of_dev->dev, "missing fsl,fifo-depth property in %s\n", + np->full_name); + ret = -ENODEV; + goto error; + } + ssi_private->fifo_depth = *iprop; + /* Initialize the the device_attribute structure */ dev_attr = &ssi_private->dev_attr; dev_attr->attr.name = "statistics";
On Wed, Aug 04, 2010 at 03:04:24PM -0500, Timur Tabi wrote:
Add code that programs the DMA and SSI controllers differently based on the FIFO depth of the SSI.
The SSI devices on the MPC8610 and the P1022 are identical in every way except one: the transmit and receive FIFO depth. On the MPC8610, the depth is eight. On the P1022, it's fifteen. The device tree nodes for the SSI include a "fsl,fifo-depth" property that specifies the FIFO depth.
My first thought with this is that it's not something I'd expect to be in the device tree at all - it looks like it's all properties of the silicon which I'd expect the drivers to be able to figure out for themselves without needing to be told by the device tree.
On Thu, Aug 5, 2010 at 6:10 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
My first thought with this is that it's not something I'd expect to be in the device tree at all - it looks like it's all properties of the silicon which I'd expect the drivers to be able to figure out for themselves without needing to be told by the device tree.
There is no way for the software to know what the FIFO depth is, so this is the sort of thing that should be in the device tree.
On Thu, Aug 05, 2010 at 08:14:43AM -0500, Timur Tabi wrote:
On Thu, Aug 5, 2010 at 6:10 AM, Mark Brown
My first thought with this is that it's not something I'd expect to be in the device tree at all - it looks like it's all properties of the silicon which I'd expect the drivers to be able to figure out for themselves without needing to be told by the device tree.
There is no way for the software to know what the FIFO depth is, so this is the sort of thing that should be in the device tree.
Are you saying that every DT for a system doing audio on a PowerPC system has to manually specify the depth of the FIFO on the silicon? That doesn't sound particularly sensible, and I had been under the impression that DT systems had a better way of coping with this.
On Aug 5, 2010, at 8:52 AM, "Mark Brown" broonie@opensource.wolfsonmicro.com wrote:
Are you saying that every DT for a system doing audio on a PowerPC system has to manually specify the depth of the FIFO on the silicon?
Only for SSI devices.
That doesn't sound particularly sensible, and I had been under the impression that DT systems had a better way of coping with this.
The whole point behind having a device tree is to specify device information that cannot be probed. So every piece of information in the DT is something that the driver needs to be told because there is no way to query the hardware.
On Thu, Aug 05, 2010 at 09:10:08AM -0500, Tabi Timur-B04825 wrote:
On Aug 5, 2010, at 8:52 AM, "Mark Brown" broonie@opensource.wolfsonmicro.com wrote:
Are you saying that every DT for a system doing audio on a PowerPC system has to manually specify the depth of the FIFO on the silicon?
Only for SSI devices.
Right, but that's the audio controller on the overwhelming majority of PowerPC devices.
That doesn't sound particularly sensible, and I had been under the impression that DT systems had a better way of coping with this.
The whole point behind having a device tree is to specify device information that cannot be probed. So every piece of information in the DT is something that the driver needs to be told because there is no way to query the hardware.
This seems crazy, it means that we're not able to use new support for hardware features to the driver which require any kind of flag or data without also going through and updating the device trees for all existing boards. That doesn't seem terribly helpful.
I'm fairly sure that some of the previous discussion with other device tree people suggested that this was something that there was infrastructure to cope with.
Mark Brown wrote:
The whole point behind having a device tree is to specify device information that cannot be probed. So every piece of information in the DT is something that the driver needs to be told because there is no way to query the hardware.
This seems crazy, it means that we're not able to use new support for hardware features to the driver which require any kind of flag or data without also going through and updating the device trees for all existing boards. That doesn't seem terribly helpful.
Hey, what do you want me to do? That's just the way the hardware is designed. It is not possible for software to know the FIFO depth of the SSI on its own, because there is no register that can be queried. The device tree was designed specifically for this purpose -- the provide a data structure (e.g. not kernel code) that describes all of the unprobe-able features of the various devices on the SOC.
I'm fairly sure that some of the previous discussion with other device tree people suggested that this was something that there was infrastructure to cope with.
I have no idea what you're talking about.
On Thu, Aug 05, 2010 at 08:14:55AM -0700, Tabi Timur-B04825 wrote:
Mark Brown wrote:
This seems crazy, it means that we're not able to use new support for hardware features to the driver which require any kind of flag or data without also going through and updating the device trees for all existing boards. That doesn't seem terribly helpful.
Hey, what do you want me to do? That's just the way the hardware is designed. It is not possible for software to know the FIFO depth of the SSI on its own, because there is no register that can be queried. The device tree was designed specifically for this purpose -- the provide a data structure (e.g. not kernel code) that describes all of the unprobe-able features of the various devices on the SOC.
This doesn't seem like a hardware issue, it seems like an issue with the way we've deployed the device tree. I'd have strongly expected that the device tree was able to incoporate all the properties that are standard to the CPU by reference somehow (with that data being distributed separately to the per system device tree).
Mark Brown wrote:
This doesn't seem like a hardware issue, it seems like an issue with the way we've deployed the device tree.
But the FIFO depth *is* a feature of the hardware. The SSI on the 8610 has a depth of 8, and the SSI on the 1022 is identical in every way except that its FIFO depth is 15. Since the FIFO watermark needs to be programmed based on the FIFO depth, the drivers need to know that information. The only way to pass that info to the drivers is via the device tree.
I'd have strongly expected that the device tree was able to incoporate all the properties that are standard to the CPU by reference somehow (with that data being distributed separately to the per system device tree).
Separately? Why? Look at the device trees we have today. They are full of nodes with all sorts of device-specific properties that describe minor differences among the devices across various SOCs.
On Thu, Aug 05, 2010 at 09:21:15AM -0700, Tabi Timur-B04825 wrote:
the FIFO depth, the drivers need to know that information. The only way to pass that info to the drivers is via the device tree.
Clearly this is not entirely true, people manage to write non-DT kernels after all. In a strongly device tree oriented model it does need to be via the device tree, but even there there's options for how the data gets there - for example, we can supply it by looking at the CPU type and fixing up the DTS prior to the driver seeing it, or any one of a number of other methods. I'd probably not have queried this so much if I'd noticed some sort of handling for such a method.
It's not so much this particular property I'm worried about as the general style for things like this so CCing in Grant.
Mark Brown wrote:
I'd have strongly expected that the device tree was able to incoporate all the properties that are standard to the CPU by reference somehow (with that data being distributed separately to the per system device tree).
Separately? Why? Look at the device trees we have today. They are full of nodes with all sorts of device-specific properties that describe minor differences among the devices across various SOCs.
Separately because people make and distribute systems with a parts on them well before software is finalised in mainline. Requiring that both the DTS and the kernel match up and get updated for new feature support means that you've got two different things (potentially maintained by different groups in the end user environment) to distribute and get updated on systems separately. Failure to update one may result in problems on another, either with features not getting enabled or with newer kernel versions not wanting to run on systems with older device trees.
It also seems like busy work for users to have to add things like this to their device trees. I think what I'd expect to see in a strongly device tree model is something like the tree for a board saying it's using a given SoC and then a standard device tree for the SoC which is shared between all the different users of that SoC getting merged in early in boot (probably from the kernel). That way if we gain support for a new feature on the SoC or discover something that needs to be flagged up for workaround then every board using the SoC will pick it up. This seems particularly useful for things like crypto engines that are physically internal to the SoC and so don't normally require per-board hookup. In ASoC terms you do need board specific hookup so that's a bit less of an issue but it looses us some of the benefit of having standard chip drivers by pushing some of the chip generic knowledge into a per-machine location.
The fact that the PowerPC systems are currently doing this isn't something I'd rely on with other architectures, the vendor and customer bases may be very different.
On Thu, Aug 5, 2010 at 12:18 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
It also seems like busy work for users to have to add things like this to their device trees. I think what I'd expect to see in a strongly device tree model is something like the tree for a board saying it's using a given SoC and then a standard device tree for the SoC which is shared between all the different users of that SoC getting merged in early in boot (probably from the kernel). That way if we gain support for a new feature on the SoC or discover something that needs to be flagged up for workaround then every board using the SoC will pick it up. This seems particularly useful for things like crypto engines that are physically internal to the SoC and so don't normally require per-board hookup. In ASoC terms you do need board specific hookup so that's a bit less of an issue but it looses us some of the benefit of having standard chip drivers by pushing some of the chip generic knowledge into a per-machine location.
The lack of shared soc data in device trees is indeed a problem that has been on my radar for a while now. Fortunately I do have a solution[1] which is partially implemented plus a contractual obligation to deliver it to a client in the near future. I fully expect this will become a non-issue between now and about mid November.
g.
[1] http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg00680.htm...
On Thu, Aug 5, 2010 at 2:43 PM, Grant Likely grant.likely@secretlab.ca wrote:
The lack of shared soc data in device trees is indeed a problem that has been on my radar for a while now. Fortunately I do have a solution[1] which is partially implemented plus a contractual obligation to deliver it to a client in the near future. I fully expect this will become a non-issue between now and about mid November.
g.
[1] http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg00680.htm...
I haven't read through all of it, but it looks like your solution affects only DTS files. I think Mark's concern is mostly with DTB files, because we still need to have a unique DTB for every possible board variation.
Am I reading that correctly?
On Thu, Aug 5, 2010 at 3:13 PM, Timur Tabi timur@freescale.com wrote:
On Thu, Aug 5, 2010 at 2:43 PM, Grant Likely grant.likely@secretlab.ca wrote:
The lack of shared soc data in device trees is indeed a problem that has been on my radar for a while now. Fortunately I do have a solution[1] which is partially implemented plus a contractual obligation to deliver it to a client in the near future. I fully expect this will become a non-issue between now and about mid November.
g.
[1] http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg00680.htm...
I haven't read through all of it, but it looks like your solution affects only DTS files. I think Mark's concern is mostly with DTB files, because we still need to have a unique DTB for every possible board variation.
Am I reading that correctly?
You are; but the lack of dts factorization must be solved first before looking at whether or not .dtb overlays make sense. Otherwise we don't have a source for the factorized data. I personally don't think .dtb overlays are needed, but I'm not closed to the idea either.
g.
On 5 Aug 2010, at 22:19, Grant Likely wrote:
You are; but the lack of dts factorization must be solved first before looking at whether or not .dtb overlays make sense. Otherwise we don't have a source for the factorized data. I personally don't think .dtb overlays are needed, but I'm not closed to the idea either.
I think all the issues from the recent discussion about bootloaders also make it strongly desirable to get the SoC generic stuff out of the DTS that's shipped separately to the kernel. You've got issues on reference boards where the SoC vendor may have shipped a low quality device tree in flash and users are scared to try to reflash the board due to a lack of recovery facilities (so the less we rely on being able to update data flashed into the device the better) and you've got issues on end user projects with large, potentially distributed, teams where coordinating updates between everyone can be tricky especially if those updates need to be done in lockstep (so the less data is shipped outside the kernel the better).
Remember rmk's constant complaint about bootloaders on ARM - these are programs that need to get two registers right on kernel start and don't reliably do so.
On Thu, Aug 5, 2010 at 3:42 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On 5 Aug 2010, at 22:19, Grant Likely wrote:
You are; but the lack of dts factorization must be solved first before looking at whether or not .dtb overlays make sense. Otherwise we don't have a source for the factorized data. I personally don't think .dtb overlays are needed, but I'm not closed to the idea either.
I think all the issues from the recent discussion about bootloaders also make it strongly desirable to get the SoC generic stuff out of the DTS that's shipped separately to the kernel...
The board specific data has the same issues as the soc specific data in this regard, so I don't think it helps much to split it up at the .dtb level. However....
... You've got issues on reference boards where the SoC vendor may have shipped a low quality device tree in flash and users are scared to try to reflash the board due to a lack of recovery facilities (so the less we rely on being able to update data flashed into the device the better) and you've got issues on end user projects with large, potentially distributed, teams where coordinating updates between everyone can be tricky especially if those updates need to be done in lockstep (so the less data is shipped outside the kernel the better).
Absolutely correct. All of this discussion and lessons learned has distilled the fact the .dtb file must not be linked into the boot firmware. Any design that requires a boot firmware upgrade to update/replace the .dtb is broken. There will still be broken cases, but fortunately for the broken cases the .dtb can always be linked into the kernel image. So, for "good" firmware, the .dtb use case works really well. In the "broken" firmware case, the .dtb(s) can be linked into the wrapper image and the correct one can be chosen based on machine-id or some other discernible board data. I'm being very careful to make sure that using a .dtb doesn't paint anybody into a corner.
Remember rmk's constant complaint about bootloaders on ARM - these are programs that need to get two registers right on kernel start and don't reliably do so.
Understood. It is my priority to complete a full-featured sample implementation so I can prove that it is actually a useful solution. I'm going to start with U-Boot and make sure upstream u-boot always passes the correct thing for passing a .dtb
g.
On 5 Aug 2010, at 23:04, Grant Likely wrote:
On Thu, Aug 5, 2010 at 3:42 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On 5 Aug 2010, at 22:19, Grant Likely wrote:
You are; but the lack of dts factorization must be solved first before looking at whether or not .dtb overlays make sense. Otherwise we don't have a source for the factorized data. I personally don't think .dtb overlays are needed, but I'm not closed to the idea either.
I think all the issues from the recent discussion about bootloaders also make it strongly desirable to get the SoC generic stuff out of the DTS that's shipped separately to the kernel...
The board specific data has the same issues as the soc specific data in this regard, so I don't think it helps much to split it up at the .dtb level. However....
This is purely on the basis that the error rate and the volume of data are correlated.
Absolutely correct. All of this discussion and lessons learned has distilled the fact the .dtb file must not be linked into the boot firmware. Any design that requires a boot firmware upgrade to update/replace the .dtb is broken. There will still be broken cases, but fortunately for the broken cases the .dtb can always be linked into the kernel image. So, for "good" firmware, the .dtb use case works really well. In the "broken" firmware case, the .dtb(s) can be linked into the wrapper image and the correct one can be chosen based on machine-id or some other discernible board data. I'm being very careful to make sure that using a .dtb doesn't paint anybody into a corner.
On the other hand if you do have to link the device tree into the wrapper image you run into fun and games with any per system data like MAC addresses which might be embedded in the device tree.
Also, it's not just good and bad firmware that's the issue - like I say, there are also coordination issues within large development teams to consider.
On Thu, Aug 5, 2010 at 4:34 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On 5 Aug 2010, at 23:04, Grant Likely wrote:
On Thu, Aug 5, 2010 at 3:42 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On 5 Aug 2010, at 22:19, Grant Likely wrote:
You are; but the lack of dts factorization must be solved first before looking at whether or not .dtb overlays make sense. Otherwise we don't have a source for the factorized data. I personally don't think .dtb overlays are needed, but I'm not closed to the idea either.
I think all the issues from the recent discussion about bootloaders also make it strongly desirable to get the SoC generic stuff out of the DTS that's shipped separately to the kernel...
The board specific data has the same issues as the soc specific data in this regard, so I don't think it helps much to split it up at the .dtb level. However....
This is purely on the basis that the error rate and the volume of data are correlated.
Absolutely correct. All of this discussion and lessons learned has distilled the fact the .dtb file must not be linked into the boot firmware. Any design that requires a boot firmware upgrade to update/replace the .dtb is broken. There will still be broken cases, but fortunately for the broken cases the .dtb can always be linked into the kernel image. So, for "good" firmware, the .dtb use case works really well. In the "broken" firmware case, the .dtb(s) can be linked into the wrapper image and the correct one can be chosen based on machine-id or some other discernible board data. I'm being very careful to make sure that using a .dtb doesn't paint anybody into a corner.
On the other hand if you do have to link the device tree into the wrapper image you run into fun and games with any per system data like MAC addresses which might be embedded in the device tree.
Also, it's not just good and bad firmware that's the issue - like I say, there are also coordination issues within large development teams to consider.
I'm starting a working group to prepare a set of requirements, recommendations and sample implementation for an ARM boot architecture. Particularly useful for more-or-less general purpose OSes like Ubuntu on ARM. We'll be talking about these kinds of issues. Would you like me to cc you when I kick it off?
On Thu, Aug 05, 2010 at 05:19:44PM -0600, Grant Likely wrote:
I'm starting a working group to prepare a set of requirements, recommendations and sample implementation for an ARM boot architecture. Particularly useful for more-or-less general purpose OSes like Ubuntu on ARM. We'll be talking about these kinds of issues. Would you like me to cc you when I kick it off?
Sure, go ahead.
On Thu, Aug 05, 2010 at 04:04:06PM -0600, Grant Likely wrote:
On Thu, Aug 5, 2010 at 3:42 PM, Mark Brown
i think all the issues from the recent discussion about bootloaders also make it strongly desirable to get the soc generic stuff out of the dts that's shipped separately to the kernel...
The board specific data has the same issues as the soc specific data in this regard, so I don't think it helps much to split it up at the .dtb level. However....
Oh, this is purely on the basis that the error count is related to the amount of data. The board specific stuff does at least have more chance someone actually looked at it on this particular board too.
Absolutely correct. All of this discussion and lessons learned has distilled the fact the .dtb file must not be linked into the boot firmware. Any design that requires a boot firmware upgrade to update/replace the .dtb is broken. There will still be broken cases, but fortunately for the broken cases the .dtb can always be linked into the kernel image. So, for "good" firmware, the .dtb use case works really well. In the "broken" firmware case, the .dtb(s) can be linked into the wrapper image and the correct one can be chosen based on machine-id or some other discernible board data. I'm being very careful to make sure that using a .dtb doesn't paint anybody into a corner.
On the other hand if you end up having to replace the device tree with one in the wrapper then you've got an issue with any per-system data like MAC addresses that someone put in the device tree unless you can do some sort of overlay/fixup thing.
Remember rmk's constant complaint about bootloaders on ARM - these are programs that need to get two registers right on kernel start and don't reliably do so.
Understood. It is my priority to complete a full-featured sample implementation so I can prove that it is actually a useful solution. I'm going to start with U-Boot and make sure upstream u-boot always passes the correct thing for passing a .dtb
u-boot will probably be fine anyway, it's more the less widely used ones that people cook up which are a concern here.
participants (4)
-
Grant Likely -
Mark Brown -
Tabi Timur-B04825 -
Timur Tabi