Re: [PATCH 10/11] usb: host: xhci-tegra: Add Tegra234 XHCI support

From: Wayne Chang
Date: Thu Nov 03 2022 - 07:35:43 EST




On 11/1/22 22:53, Jon Hunter wrote:
> On 28/10/2022 14:39, Thierry Reding wrote:
>
> ...
>
>>> +static const struct tegra_xusb_soc tegra234_soc = {
>>> +    .firmware = "nvidia/tegra234/xusb.bin",
>>> +    .supply_names = tegra194_supply_names,
>>> +    .num_supplies = ARRAY_SIZE(tegra194_supply_names),
>>> +    .phy_types = tegra194_phy_types,
>>> +    .num_types = ARRAY_SIZE(tegra194_phy_types),
>>> +    .context = &tegra186_xusb_context,
>>> +    .ports = {
>>> +        .usb3 = { .offset = 0, .count = 4, },
>>> +        .usb2 = { .offset = 4, .count = 4, },
>>> +    },
>>> +    .scale_ss_clock = false,
>>> +    .has_ipfs = false,
>>> +    .otg_reset_sspi = false,
>>> +    .ops = &tegra234_ops,
>>> +    .mbox = {
>>> +        .cmd = XUSB_BAR2_ARU_MBOX_CMD,
>>> +        .data_in = XUSB_BAR2_ARU_MBOX_DATA_IN,
>>> +        .data_out = XUSB_BAR2_ARU_MBOX_DATA_OUT,
>>> +        .owner = XUSB_BAR2_ARU_MBOX_OWNER,
>>> +        .smi_intr = XUSB_BAR2_ARU_SMI_INTR,
>>> +    },
>>> +    .lpm_support = true,
>>> +    .has_bar2 = true,
>>> +    .has_ifr = true,
>>> +};
>>> +MODULE_FIRMWARE("nvidia/tegra234/xusb.bin");
>>
>> Can you prepare a patch to add this firmware to the linux-firmware
>> repository? I don't see it there yet.
>
>
> Actually, we should remove the MODULE_FIRMWARE completely for Tegra234.
> Per the commit message the variable 'has_ifr' is used to indicate if the
> firmware is loaded by calling request_firmware() or via these IFR
> registers. I wonder if we need this 'has_ifr' variable if we should just
> avoid setting the 'firmware' variable for Tegra234 and use this instead
> of the 'has_ifr'?
>

Thanks for the review.

Yes, correct. The firmware loading is moved to MB2 and thus we do not
need it anymore. I'll remove it together with the .firmware in the soc data.

We could checking firmware in soc data instead of has_ifr as we now only
get two ways to load the firmware.
I'll make the change on it in the next patch series

thanks,
Wayne.

> Jon
>