Re: [PATCH 1/4] thunderbolt: add platform nhi driver

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 11/7/26 8:04 PM, Sven Peter wrote:
>> Add a new file plat.c to register platform NHI driver for MediaTek
>> mt8901/mt8992 SoCs.
>>
>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>
> 
> [...]
> 
>> +
>> +static void nhi_plat_shutdown(struct platform_device *pdev)
>> +{
>> +	nhi_plat_do_remove(pdev, true);
>> +}
>> +
>> +
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id nhi_plat_of_match[] = {
>> +	{ .compatible = "thunderbolt-platform" },
> 
> Do you already have hardware that probes via device tree? You'll have to
> write bindings and get them reviewed before adding any compatible here.
> Generic ones like you used here aren't usually allowed.
> 
> If you don't have a dts based platform I'd probably drop this, not much of
> this code here is e.g. re-usable for Apple since they like to think very
> different, see my series in [1] that I can pick up again now that we
> made progress on how to represent connections between the NHI and the
> protocol adapters.
> 
> Not sure about qualcom since I haven't seen a series for the NHI yet but
> I'm sure Konrad can comment there. I kinda expect that it won't have
> much shared code either.

We're in the same boat regarding thinking different. This driver is
for implementations where the underlying hardware is abstracted away
(be it through architecture or intermediate firmware) and the TBT
layer writes are enough to e.g. power sequence the connected buses,
just like on Intel.

For the OF compatible, we definitely do need bindings and I too would
like to know if this is actually exercised - if not, we should cut it
out and trivially bring it back when/if the need comes around.

> Take a look at e.g. more recent drivers in drivers/usb/dwc3: They
> usually add a dwc3-$vendor.c file that takes care of all the
> vendor-specific quirks and that ends up being nicer than a single
> platform.c file with tons of quirks and separate code paths for each and
> every vendor.

Yeah, this is a nice parallel. Adding to it, there not only exist
dwc3-$vendor drivers that take care of the additional registers and
whatnot, but also everything around the USB subsystem - PHYs, perhaps
some NoC drivers, and all the infra that feeds into there, and it
compounds back into the size of some of those dwc3 glue drivers..


That being said, the makers of the spec probably wanted/expected the
hardware to have exactly this neat level of abstraction and I
wouldn't be surprised if more designs end up being compatible. Or if
the platform firmware makes it appear so.

Konrad