Re: [PATCH] ARM: dts: qcom: msm8974: correct qfprom node reg

[Konrad Dybcio]

On 19.04.2023 18:18, Luca Weiss wrote:
> On Mittwoch, 19. April 2023 18:12:04 CEST Konrad Dybcio wrote:
>> On 19.04.2023 18:00, Luca Weiss wrote:
>>> Hi Konrad,
>>>
>>> On Montag, 30. Jänner 2023 21:37:29 CEST Luca Weiss wrote:
>>>> On Montag, 30. Jänner 2023 19:42:51 CET Konrad Dybcio wrote:
>>>>> On 30.01.2023 19:36, Luca Weiss wrote:
>>>>>> On Montag, 30. Jänner 2023 19:30:04 CET Konrad Dybcio wrote:
>>>>>>> On 30.01.2023 19:20, luca@xxxxxxxxx wrote:
>>>>>>>> From: Craig Tatlor <ctatlor97@xxxxxxxxx>
>>>>>>>>
>>>>>>>> The qfprom actually starts at 0xfc4b8000 instead of 0xfc4bc000 as
>>>>>>>> defined previously. Adjust the tsens offsets accordingly.
>>>>>>>>
>>>>>>>> [luca@xxxxxxxxx: extract to standalone patch]
>>>>>>>>
>>>>>>>> Fixes: c59ffb519357 ("arm: dts: msm8974: Add thermal zones, tsens and
>>>>>>>> qfprom nodes") Signed-off-by: Craig Tatlor <ctatlor97@xxxxxxxxx>
>>>>>>>> Signed-off-by: Luca Weiss <luca@xxxxxxxxx>
>>>>>>>> ---
>>>>>>>
>>>>>>> Isn't this a raw vs ecc-corrected values problem?
>>>>>>
>>>>>> Not quite sure what you mean.
>>>>>
>>>>> The QFPROM is split into two parts: one where raw values
>>>>> are stored, and the other one where ECC-corrected copies
>>>>> of them reside. Usually it's at offset of 0x4000. We should
>>>>> generally be using the ECC-corrected ones, because.. well..
>>>>> they are ECC-corrected.. You may want to check if the
>>>>> fuse you're adding reads the same value at +0x4000.
>>>>
>>>> Yeah that actually seems to work...
>>>>
>>>> But downstream's using this +0x4000 only for tsens it seems
>>>>
>>>>    <0xfc4bc000 0x1000> as "tsens_eeprom_physical"
>>>>
>>>> qcom,clock-krait-8974 is using this:
>>>>     <0xfc4b80b0 0x08> as "efuse"
>>>>
>>>> Also seems HDMI driver is using a mix for HDCP stuff
>>>>
>>>>   drivers/video/msm/mdss/mdss_hdmi_util.h:
>>>>     /* QFPROM Registers for HDMI/HDCP */
>>>>     #define QFPROM_RAW_FEAT_CONFIG_ROW0_LSB  (0x000000F8)
>>>>     #define QFPROM_RAW_FEAT_CONFIG_ROW0_MSB  (0x000000FC)
>>>>     #define HDCP_KSV_LSB                     (0x000060D8)
>>>>     #define HDCP_KSV_MSB                     (0x000060DC)
>>>>
>>>> Any clue why Qualcomm used it this way in downstream? I'd rather not
>>>> deviate too much if not for a good reason...
>>>
>>> Any comments on the above?
>>
>> This thread got burried to deep in the mailbox!
>>
>> I see two reasons why they could be using the uncorrected region:
>> - their generators are messed up in general
>>
>> - they may have had an early chip revision once where there were
>>   problems with this and their generators were messed up to
>>   accommodate for it and everybody forgot to fix that
>>
>> No other good explanations as far as I'm aware!
> 
> So, resolution is to use the offsets as declared in downstream, so take this 
> patch to have the full range available?
No, the correct resolution to "fix QFPROM reg" would be to
increase the size to 0x7000-0x4000 = 0x3000, as we should be
using the ECC-corrected entries.

Konrad
> 
> Regards
> Luca
> 
> 
>