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

Konrad Dybcio
2023-04-19T16:20:41Z

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@z3ntu.xyz wrote:
>>>>>>>> From: Craig Tatlor <ctatlor97@gmail.com>
>>>>>>>>
>>>>>>>> The qfprom actually starts at 0xfc4b8000 instead of 0xfc4bc000 as
>>>>>>>> defined previously. Adjust the tsens offsets accordingly.
>>>>>>>>
>>>>>>>> [luca@z3ntu.xyz: extract to standalone patch]
>>>>>>>>
>>>>>>>> Fixes: c59ffb519357 ("arm: dts: msm8974: Add thermal zones, tsens and
>>>>>>>> qfprom nodes") Signed-off-by: Craig Tatlor <ctatlor97@gmail.com>
>>>>>>>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
>>>>>>>> ---
>>>>>>>
>>>>>>> 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
>
>
>