On Tue, Oct 29, 2024 at 1:17 AM Krishna Chaitanya Chundru <quic_krichai@xxxxxxxxxxx> wrote: > > > > On 10/29/2024 12:21 AM, James Quinlan wrote: > > On 10/24/24 21:08, Krishna Chaitanya Chundru wrote: > >> > >> > >> On 10/22/2024 12:33 AM, Rob Herring wrote: > >>> On Fri, Oct 18, 2024 at 02:22:45PM -0400, Jim Quinlan wrote: > >>>> Support configuration of the GEN3 preset equalization settings, aka the > >>>> Lane Equalization Control Register(s) of the Secondary PCI Express > >>>> Extended Capability. These registers are of type HwInit/RsvdP and > >>>> typically set by FW. In our case they are set by our RC host bridge > >>>> driver using internal registers. > >>>> > >>>> Signed-off-by: Jim Quinlan <james.quinlan@xxxxxxxxxxxx> > >>>> --- > >>>> .../devicetree/bindings/pci/brcm,stb-pcie.yaml | 12 > >>>> ++++++++++++ > >>>> 1 file changed, 12 insertions(+) > >>>> > >>>> diff --git > >>>> a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > >>>> b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > >>>> index 0925c520195a..f965ad57f32f 100644 > >>>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > >>>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > >>>> @@ -104,6 +104,18 @@ properties: > >>>> minItems: 1 > >>>> maxItems: 3 > >>>> + brcm,gen3-eq-presets: > >>>> + description: | > >>>> + A u16 array giving the GEN3 equilization presets, one for > >>>> each lane. > >>>> + These values are destined for the 16bit registers known as the > >>>> + Lane Equalization Control Register(s) of the Secondary PCI > >>>> Express > >>>> + Extended Capability. In the array, lane 0 is first term, > >>>> lane 1 next, > >>>> + etc. The contents of the entries reflect what is necessary for > >>>> + the current board and SoC, and the details of each preset are > >>>> + described in Section 7.27.4 of the PCI base spec, Revision 3.0. > >>> > >>> If these are defined by the PCIe spec, then why is it Broadcom specific > >>> property? > > Yes, I will remove the "brcm," prefix. > >>> > >> Hi Rob, > >> > >> qcom pcie driver also needs to program these presets as you suggested > >> this can go to common pci bridge binding. > >> > >> from PCIe spec 6.0.1 revision section 8.3.3.3 & 4.2.4.2 for data rates > >> of 8.0 GT/s, 16.0 GT/s, and 32.0 GT/s uses one class of preset (P0 > >> through P10) and where as data rates of 64.0 GT/s use different class of > >> presets (Q0 through Q10) (Table 4-23). And data rates of 8.0 GT/s also > >> have optional preset hints (Table 4-24). > >> > >> And there is possibility that for each data rate we may require > >> different preset configuration. > >> > >> Can we have a dt binding for each data rate of 16 byte array. > >> like gen3-eq-preset array, gen4-eq-preset array etc. > > > > Yes, that was the idea when using "genX-eq-preset", for X in {3,4...}. > > > > Keep in mind that this is an RFC; I have a backlog of commit submissions > > before I can submit the code that uses this DT property. If you > > (Krishna) want to submit something now I'd be quite happy to go with > > that. I don't believe it is acceptable to submit a bindings commit w/o > > code that uses it (if I'm incorrect I'll be glad to do a V2). > > > Hi Jim, > > I submitted a pull request for this. if you have any other suggestions > or if we need to have any other details we can update this pull request. > https://github.com/devicetree-org/dt-schema/pull/146 Hi Krishna, Thanks for doing this. However, why a u8 array? The registers are defined as u16 so it would be more natural to use a u16 array, each entry giving all of the data for a single lane. In our implementation we read a u16 and we write it to the register -- what advantage is there by using a u8 array? Also if there are 16 lanes, you will need 32 maxItems, correct? Regards, Jim Quinlan Broadcom STB/CM > > > > - Krishna Chaitanya. > > Regards, > > > > Jim Quinlan > > Broadcom STB/CM > > > >> > >> - Krishna Chaitanya > >>>> + > >>>> + $ref: /schemas/types.yaml#/definitions/uint16-array > >>> > >>> minItems: 1 > >>> maxItems: 16 > >>> > >>> Last I saw, you can only have up to 16 lanes. > >>> > >>> Rob > >>> > >
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature