linux-kernel - Re: [PATCH 1/1] RFC: dt bindings: Add property "brcm,gen3-eq-presets"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+-6iNx1SA9iiECS1O=cwkcu2KWcPvvB7Zvm7=SR3F5m9nv+fg@mail.gmail.com>
Date: Tue, 29 Oct 2024 10:22:36 -0400
From: Jim Quinlan <james.quinlan@...adcom.com>
To: Krishna Chaitanya Chundru <quic_krichai@...cinc.com>
Cc: Rob Herring <robh@...nel.org>, linux-pci@...r.kernel.org, 
	Nicolas Saenz Julienne <nsaenz@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Lorenzo Pieralisi <lorenzo.pieralisi@....com>, bcm-kernel-feedback-list@...adcom.com, 
	jim2101024@...il.com, Florian Fainelli <florian.fainelli@...adcom.com>, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kw@...ux.com>, 
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, 
	"moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE" <linux-arm-kernel@...ts.infradead.org>, 
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@...ts.infradead.org>, 
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] RFC: dt bindings: Add property "brcm,gen3-eq-presets"

On Tue, Oct 29, 2024 at 1:17 AM Krishna Chaitanya Chundru
<quic_krichai@...cinc.com> 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@...adcom.com>
> >>>> ---
> >>>>   .../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
> >>>
> >

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4210 bytes)