schemas: pci: bridge: Document PCIe equalization presets

[krishnachaitanya-chundru]

PCIe equalization presets are predefined settings used to optimize signal integrity by compensating for signal loss and distortion in high-speed data transmission.

As per PCIe spec 6.0.1 revision section 8.3.3.3 & 4.2.4 for data rates of 8.0 GT/s, 16.0 GT/s, 32.0 GT/s and 64.0 GT/s provides a way to configure lane equalization presets for each lane to enhance the PCIe link reliability. Each preset value represents a different combination of pre-shoot and de-emphasis values.

For each data rate it may be required to have a different preset configuration and each lane will have different preset values.

Define per data rate equalization preset property with array of 16 representing 16 lanes of the PCIe.

[robherring]

Looks fine to me. @bjorn-helgaas ?

[bjorn-helgaas]

On Mon, Nov 4, 2024 at 6:54 AM Rob Herring ***@***.***> wrote: Looks fine to me. @bjorn-helgaas <https://github.com/bjorn-helgaas> ?

OK by me. I suppose it's obvious to driver writers (though it caused confusion for me), but is there any reason not to be explicit in the schema descriptions about where these values go, i.e., eq-presets-8gts values go in Lane Equalization Control registers (PCIe r6.0, sec 7.7.3.4), eq-presets-16gts values go in 16.0 GT/s Lane Equalization Control registers (sec 7.7.5.9), etc? To me, spec citations would be more useful than the "Each preset value is used to adjust the transmitter settings to improve signal quality and meet the electrical requirements" text. Without the details from the spec, that text isn't enough to help figure out the numbers to put in the array or where to program them. Do these patches and discussion get archived anywhere, e.g., on lore, or is it only on github?

[robherring]

On Mon, Nov 4, 2024 at 6:54 AM Rob Herring @.***> wrote: Looks fine to me. @bjorn-helgaas https://github.com/bjorn-helgaas ?
OK by me. I suppose it's obvious to driver writers (though it caused confusion for me), but is there any reason not to be explicit in the schema descriptions about where these values go, i.e., eq-presets-8gts values go in Lane Equalization Control registers (PCIe r6.0, sec 7.7.3.4), eq-presets-16gts values go in 16.0 GT/s Lane Equalization Control registers (sec 7.7.5.9), etc? To me, spec citations would be more useful than the "Each preset value is used to adjust the transmitter settings to improve signal quality and meet the electrical requirements" text. Without the details from the spec, that text isn't enough to help figure out the numbers to put in the array or where to program them.

Makes sense to me to add that.

I would hope there's exactly 1 implementation stuffing these values into registers and not leaving it up to each driver to implement (and therefore get it wrong).

Do these patches and discussion get archived anywhere, e.g., on lore, or is it only on github?

Only on GH. I do take dtschema patches from devicetree-spec list as well if submitter prefer email.

[krishnachaitanya-chundru]

On Mon, Nov 4, 2024 at 6:54 AM Rob Herring @.***> wrote: Looks fine to me. @bjorn-helgaas https://github.com/bjorn-helgaas ?
OK by me. I suppose it's obvious to driver writers (though it caused confusion for me), but is there any reason not to be explicit in the schema descriptions about where these values go, i.e., eq-presets-8gts values go in Lane Equalization Control registers (PCIe r6.0, sec 7.7.3.4), eq-presets-16gts values go in 16.0 GT/s Lane Equalization Control registers (sec 7.7.5.9), etc? To me, spec citations would be more useful than the "Each preset value is used to adjust the transmitter settings to improve signal quality and meet the electrical requirements" text. Without the details from the spec, that text isn't enough to help figure out the numbers to put in the array or where to program them. Do these patches and discussion get archived anywhere, e.g., on lore, or is it only on github?

I will update the description as suggested in next patch.

[krishnachaitanya-chundru]

On Mon, Nov 4, 2024 at 6:54 AM Rob Herring @.***> wrote: Looks fine to me. @bjorn-helgaas https://github.com/bjorn-helgaas ?
OK by me. I suppose it's obvious to driver writers (though it caused confusion for me), but is there any reason not to be explicit in the schema descriptions about where these values go, i.e., eq-presets-8gts values go in Lane Equalization Control registers (PCIe r6.0, sec 7.7.3.4), eq-presets-16gts values go in 16.0 GT/s Lane Equalization Control registers (sec 7.7.5.9), etc? To me, spec citations would be more useful than the "Each preset value is used to adjust the transmitter settings to improve signal quality and meet the electrical requirements" text. Without the details from the spec, that text isn't enough to help figure out the numbers to put in the array or where to program them.

Makes sense to me to add that.

I would hope there's exactly 1 implementation stuffing these values into registers and not leaving it up to each driver to implement (and therefore get it wrong).

Rob, these programming needs to be done before initiating link training, I don't we can have a common implementation for all since at the time of programming these registers pci_dev is not created to use common implementation.

Do these patches and discussion get archived anywhere, e.g., on lore, or is it only on github?

Only on GH. I do take dtschema patches from devicetree-spec list as well if submitter prefer email.

@bjorn-helgaas, let me know which way you prefer.

[bjorn-helgaas]

On Mon, Nov 04, 2024 at 09:22:59PM -0800, Krishna chaitanya chundru wrote: &gt; &gt; On Mon, Nov 4, 2024 at 6:54 AM Rob Herring _**@**_.***&gt; wrote: Looks fine to me. @bjorn-helgaas https://github.com/bjorn-helgaas ? &gt; &gt; OK by me. I suppose it&#39;s obvious to driver writers (though it caused confusion for me), but is there any reason not to be explicit in the schema descriptions about where these values go, i.e., eq-presets-8gts values go in Lane Equalization Control registers (PCIe r6.0, sec 7.7.3.4), eq-presets-16gts values go in 16.0 GT/s Lane Equalization Control registers (sec 7.7.5.9), etc? To me, spec citations would be more useful than the &quot;Each preset value is used to adjust the transmitter settings to improve signal quality and meet the electrical requirements&quot; text. Without the details from the spec, that text isn&#39;t enough to help figure out the numbers to put in the array or where to program them. &gt; &gt; Makes sense to me to add that. &gt; &gt; I would hope there&#39;s exactly 1 implementation stuffing these values into registers and not leaving it up to each driver to implement (and therefore get it wrong). Rob, these programming needs to be done before initiating link training, I don&#39;t we can have a common implementation for all since at the time of programming these registers pci_dev is not created to use common implementation. &gt; &gt; &gt; Do these patches and discussion get archived anywhere, e.g., on lore, or is it only on github? &gt; &gt; Only on GH. I do take dtschema patches from devicetree-spec list as well if submitter prefer email. @bjorn-helgaas, let me know which way you prefer.

Thanks for the pointer to ***@***.*** I'm not really involved in dtschema stuff, so don't really have standing to express a preference. I asked because lore is incredibly useful as a permanent, searchable, scriptable, public archive. GitHub is also useful but in different ways. Sorry about the ugly quote formatting above. I guess GitHub wasn't really built with plain-text email in mind.

[robherring]

On Mon, Nov 4, 2024 at 6:54 AM Rob Herring @.***> wrote: Looks fine to me. @bjorn-helgaas https://github.com/bjorn-helgaas ?
OK by me. I suppose it's obvious to driver writers (though it caused confusion for me), but is there any reason not to be explicit in the schema descriptions about where these values go, i.e., eq-presets-8gts values go in Lane Equalization Control registers (PCIe r6.0, sec 7.7.3.4), eq-presets-16gts values go in 16.0 GT/s Lane Equalization Control registers (sec 7.7.5.9), etc? To me, spec citations would be more useful than the "Each preset value is used to adjust the transmitter settings to improve signal quality and meet the electrical requirements" text. Without the details from the spec, that text isn't enough to help figure out the numbers to put in the array or where to program them.

Makes sense to me to add that.
I would hope there's exactly 1 implementation stuffing these values into registers and not leaving it up to each driver to implement (and therefore get it wrong).

Rob, these programming needs to be done before initiating link training, I don't we can have a common implementation for all since at the time of programming these registers pci_dev is not created to use common implementation.

The minimum is 1 common function to read the properties. That's what we have for all common properties such as link speed. Ideally, it would just plug the values into the registers as well, but perhaps the config space accessors are not available at that point. Really, I'd like to see the core managing link training as well (with callbacks to handle the h/w specifics). It is possible for link training to happen later (after probe). That happens already on some controller drivers.

Anyways, that's all separate from this patch.

Do these patches and discussion get archived anywhere, e.g., on lore, or is it only on github?

Only on GH. I do take dtschema patches from devicetree-spec list as well if submitter prefer email.

@bjorn-helgaas, let me know which way you prefer.

Please don't switch in the middle of this.

[bjorn-helgaas]

I would hope there's exactly 1 implementation stuffing these values into registers and not leaving it up to each driver to implement (and therefore get it wrong).

Rob, these programming needs to be done before initiating link training, I don't we can have a common implementation for all since at the time of programming these registers pci_dev is not created to use common implementation.

The minimum is 1 common function to read the properties. That's what we have for all common properties such as link speed. Ideally, it would just plug the values into the registers as well, but perhaps the config space accessors are not available at that point. Really, I'd like to see the core managing link training as well (with callbacks to handle the h/w specifics). It is possible for link training to happen later (after probe). That happens already on some controller drivers.

I think the few drivers that program these registers (tegra194, qcom, mediatek-gen3) do it via device-specific registers, not via the architected config space ones. The registers in config space are mostly HwInit/RO so likely not writable in general.