Adding Bjorn to the To ...
On 14/06/2021 21:37, Jon Hunter wrote:
> Hi Bjorn, Lorenzo,
>
> On 10/06/2021 10:07, Thierry Reding wrote:
>> On Thu, Jun 10, 2021 at 07:41:34AM +0100, Jon Hunter wrote:
>>> Commit 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM
>>> errata") caused a few build regressions for the Tegra194 PCIe driver
>>> which are:
>>>
>>> 1. The Tegra194 PCIe driver can no longer be built as a module. This
>>> was caused by removing the Makefile entry to build the pcie-tegra.c
>>> based upon the CONFIG_PCIE_TEGRA194 option. Therefore, restore this
>>> so that we can build the driver as a module.
>>> 2. 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM
>>> errata") added "#ifdef CONFIG_PCIE_TEGRA194" around the native
>>> driver. But if we set CONFIG_PCIE_TEGRA194=m to build the driver as a
>>> module, autoconf.h contains "#define CONFIG_PCIE_TEGRA194_MODULE 1"
>>> (not "#define CONFIG_PCIE_TEGRA194 1"), so the #ifdef excludes the
>>
>> I think the correct way to write conditionals that are true on =y and =m
>> is to use:
>>
>> #ifdef IS_ENABLED(CONFIG_PCIE_TEGRA194)
>>
>> but that doesn't fix the issue of not getting the quirks included when
>> CONFIG_PCIE_TEGRA194=m.
>>
>>> driver. Given that the ACPI quirk code for Tegra194 is completely
>>> independent of the native Tegra194 PCIe driver, move this code into
>>> its own file so that it can be built independently and we can remove
>>> the "#ifdef CONFIG_PCIE_TEGRA194" in the native driver. Note that
>>> given the native Tegra194 PCIe driver is only used with device-tree,
>>> this will not cause any conflicts.
>>> 3. The below build warnings to be seen with particular kernel
>>> configurations. Fix these by moving these structure definitions to
>>> within the necessary guards.
>>>
>>> drivers/pci/controller/dwc/pcie-tegra194.c:259:18: warning:
>>> ‘event_cntr_data_offset’ defined but not used [-Wunused-const-variable=]
>>> drivers/pci/controller/dwc/pcie-tegra194.c:250:18: warning:
>>> ‘event_cntr_ctrl_offset’ defined but not used [-Wunused-const-variable=]
>>> drivers/pci/controller/dwc/pcie-tegra194.c:243:27: warning:
>>> ‘pcie_gen_freq’ defined but not used [-Wunused-const-variable=]
>>>
>>> Fixes: 7f100744749e ("PCI: tegra: Add Tegra194 MCFG quirks for ECAM errata")
>>> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
>>> ---
>>> Changes since V3:
>>> - Moved ACPI quirk code into separate source file
>>>
>>> Changes since V2:
>>> - Update the commit message per Bjorn's feedback
>>> - Moved the structure definitions within the necessary guards as opposed
>>> to wrapping the existing defintions with the appropriate guards.
>>>
>>> Changes since V1:
>>> - Added fixes tag
>>> - Fixed 'defined but not used' compiler warnings
>>>
>>> drivers/pci/controller/dwc/Makefile | 3 +-
>>> .../pci/controller/dwc/pcie-tegra194-acpi.c | 108 ++++++++++++++
>>> drivers/pci/controller/dwc/pcie-tegra194.c | 138 +++---------------
>>> 3 files changed, 128 insertions(+), 121 deletions(-)
>>> create mode 100644 drivers/pci/controller/dwc/pcie-tegra194-acpi.c
>>
>> We've had a similar problem with Tegra210 EMC and the EMC frequency
>> table parsing code. The EMC frequency table parsing code needs to be
>> always built-in because it is stashed into a linker section. However
>> we still want the driver to be able to be a loadable module. The only
>> way I know how to achieve that is by splitting up the sources files
>> like you do here, so I this looks like a correct fix:
>>
>> Reviewed-by: Thierry Reding <treding@xxxxxxxxxx>
>
>
> Just a reminder that we need to get this fixed for v5.13. Please let me
> know if you have any more comments on this fix.
>
> Thanks!
> Jon
>
--
nvpublic
