Re: [PATCH v8 4/4] PCI: dwc: Add support for configuring lane equalization presets

[Konrad Dybcio]

On 4/2/25 8:02 AM, Manivannan Sadhasivam wrote:
> On Sat, Mar 29, 2025 at 12:42:02PM +0100, Konrad Dybcio wrote:
>> On 3/29/25 10:39 AM, Manivannan Sadhasivam wrote:
>>> On Sat, Mar 29, 2025 at 09:59:46AM +0100, Konrad Dybcio wrote:
>>>> On 3/29/25 7:30 AM, Manivannan Sadhasivam wrote:
>>>>> On Fri, Mar 28, 2025 at 10:53:19PM +0100, Konrad Dybcio wrote:
>>>>>> On 3/28/25 7:45 AM, Manivannan Sadhasivam wrote:
>>>>>>> On Fri, Mar 28, 2025 at 11:04:11AM +0530, Krishna Chaitanya Chundru wrote:

[...]

>> Ohh, I didn't think about that - and I can only think about solutions that are
>> rather janky.. with perhaps the least janky one being changing the else case I
>> proposed above into:
>>
>> else if (speed >= PCIE_SPEED_32_0GT && eq_presets_Ngts[speed - PCIE_SPEED_16_0GT][0] != PCI_EQ_RESV) {
> 
> s/PCIE_SPEED_16_0GT/PCIE_SPEED_32_0GT
> 
>> 	...
> 
> So this I read as: Oh, your controller supports 32 GT/s and you firmware also
> wanted to apply the custom preset offsets, but sorry we didn't do it because we
> don't know if it would work or not. So please let us know so that we can work
> with you test it and then finally we can apply the presets.

Good, because that was exactly what I had in mind :)

>>>>> I'm not forseeing any issue in this part of the code to support higher GEN
>>>>> speeds though.
>>>>
>>>> I would hope so as well, but both not programming and misprogramming are
>>>> equally hard to detect
>>>>
>>>
>>> I don't disagree. I wanted to have it since there is no sensible way of warning
>>> users that this part of the code needs to be updated in the future.
>>
>> I understand, however I'm worried that the programming sequence or register
>> may change for higher speeds in a way that would be incompatible with what
>> we assume here
>>
> 
> Honestly, I don't know why you are having this opinion. This piece of code is
> not in Qcom driver and the registers are the same for 8 GT/s, 16 GT/s as per the
> PCIe spec. So the hardware programming sequence and other arguments doesn't
> apply here (atleast to me).

I'm not familiar with the spec, but if you think it's a good idea to extend
the sequence for 32+ GT/s, I won't object anymore

Konrad