On 1.12.2022 11.01, Arnd Bergmann wrote:
On Thu, Dec 1, 2022, at 09:06, Greg Kroah-Hartman wrote:
On Thu, Dec 01, 2022 at 11:13:31AM +0900, Daehwan Jung wrote:
This driver works with xhci platform driver. It needs to override
functions of xhci_plat_hc_driver. Wakelocks are used for sleep/wakeup
scenario of system.
So this means that no other platform xhci driver can be supported in the
same system at the same time.
Which kind of makes sense as that's not anything a normal system would
have, BUT it feels very odd. This whole idea of "override the platform
driver" feels fragile, why not make these just real platform drivers and
have the xhci platform code be a library that the other ones can use?
That way you have more control overall, right?
Agree that overriding the generic platform driver xhci_hc_platform_driver
from this exynos driver is odd.
But I don't understand how this works.
Where are the hcds created and added when this xhci-exonys driver binds to
the device? all this driver does in probe is the overriding?
Am I missing something here?
Agreed, having another layer here (hcd -> xhci -> xhcd_platform ->
xhcd_exynos) would fit perfectly well into how other SoC specific
drivers are abstracted. This could potentially also help reduce
the amount of code duplication between other soc specific variants
(mtk, tegra, mvebu, ...) that are all platform drivers but don't
share code with xhci-plat.c.
Alternatively, it seems that all of the xhci-exynos support could
just be part of the generic xhci-platform driver: as far as I can
tell, none of the added code is exynos specific at all, instead it
is a generic xhci that is using the wakeup_source framework.
Sounds reasonable as well, and if some exynos specific code is needed
then just create a xhci_plat_priv struct for exynos and pass it in
of_device_id data like other vendors that use the generic
xhci-platform driver do.
-Mathias
