linux-kernel - Re: [PATCH RESEND v3 3/6] i3c: mipi-i3c-hci: Add a quirk to set PIO mode

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d0ca0a49-311e-4af3-95df-f7c6f85e184f@amd.com>
Date: Mon, 12 Aug 2024 15:02:03 +0530
From: Shyam Sundar S K <Shyam-sundar.S-k@....com>
To: Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc: Guruvendra Punugupati <Guruvendra.Punugupati@....com>,
 Krishnamoorthi M <krishnamoorthi.m@....com>, linux-i3c@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND v3 3/6] i3c: mipi-i3c-hci: Add a quirk to set PIO
 mode

Hi,

On 8/12/2024 14:47, Jarkko Nikula wrote:
> Hi
> 
> On 8/9/24 6:44 PM, Shyam Sundar S K wrote:
>>> I was thinking these quirks can be passed as driver_data more cleanly
>>> and be specific only to affected HW if AMD HW would have an unique
>>> ACPI ID for each HW version.
>>>
>>> Above X86_VENDOR_AMD might be too generic if and when quirks are fixed
>>> in the future HW :-)
>>>
>>> So something like:
>>>
>>> static const struct acpi_device_id i3c_hci_acpi_match[] = {
>>>      {"AMDI1234", HCI_QUIRK_PIO_MODE | HCI_QUIRK_OD_PP_TIMING |
>>> HCI_QUIRK_RESP_BUF_THLD},
>>>      {}
>>> };
>>>
>>> and set them in the i3c_hci_probe() as:
>>>
>>> hci->quirks = (unsigned long)device_get_match_data(&pdev->dev);
>>
>> Nice idea. But only problem is that MSFT wants to have the same ACPI
>> ID present in the specification.
>>
>> I have replied to Andy on patch 1/6. Can you please put your remarks
>> there?
>>
> Well this is implementation detail later in the series and I found it
> better to focus ACPI ID discussion in 1/6.
> 
>> Yeah, agreed that having X86_VENDOR_AMD is too generic, but felt its
>> good to have additional checks only after the HW is fixed, rather than
>> being speculative now.. :-)
>>
>> What would you advise?
>> Most probably there will be future HW with either exactly same set of 
> quirks, reduced quirks or new quirks and X86_VENDOR_AMD test will work
> only with the first case :-)

I can add an additional check with the CPU ID and distinguish them(so
the quirk gets applied to the affected HW versions) and just not
restrict to X86_VENDOR_AMD, would that be fine with you?

OTOH, Since these are quirks (where its a broken hardware problems)
and the idea you suggested is related to driver data (where driver
data is meant to store private information about the device)

static const struct acpi_device_id i3c_hci_acpi_match[] = {
     {"AMDI1234", HCI_QUIRK_PIO_MODE | HCI_QUIRK_OD_PP_TIMING |
HCI_QUIRK_RESP_BUF_THLD},
     {}
};

does that not conflict? quirk vs driver data?

I am OK to implement it the way you prefer :-)

Thanks,
Shyam