Re: [PATCH v2 4/4] driver core: platform: use generic driver_override infrastructure
Frank Li <[email protected]>
| Newsgroups | gmane.linux.kernel.hwmon,gmane.linux.kernel,gmane.linux.ports.arm.msm,gmane.linux.sound,gmane.linux.ports.sh.devel |
|---|---|
| Message-ID | <[email protected]> |
From: Frank Li (AI-BOT) <[email protected]> AI bot review and may be useless. > @@ -25,10 +25,12 @@ static int platform_match(struct device *dev, struct device_driver *drv) > { > struct platform_device *pdev = to_platform_device(dev); > struct platform_driver *pdrv = to_platform_driver(drv); > + int ret; > > /* When driver_override is set, only bind to the matching driver */ > - if (pdev->driver_override) > - return !strcmp(pdev->driver_override, drv->name); > + ret = device_match_driver_override(dev, drv); > + if (ret >= 0) > + return ret; Good: centralizes locking logic. Verify device_match_driver_override() handles -1 return for "not set" case consistently across all callers. > @@ -603,7 +603,6 @@ static void platform_device_release(struct device *dev) > kfree(pa->pdev.dev.platform_data); > kfree(pa->pdev.mfd_cell); > kfree(pa->pdev.resource); > - kfree(pa->pdev.driver_override); Correct: driver_override now managed by generic infrastructure, not platform_device. Ensure device_release() in driver-core handles cleanup. > -static ssize_t driver_override_show(struct device *dev, > - struct device_attribute *attr, char *buf) > -{ > - struct platform_device *pdev = to_platform_device(dev); > - ssize_t len; > - > - device_lock(dev); > - len = sysfs_emit(buf, "%s\n", pdev->driver_override); > - device_unlock(dev); > - > - return len; > -} Good: removes duplicate sysfs attribute. Verify generic implementation in device-core provides equivalent locking and sysfs_emit() behavior. > @@ -1377,10 +1347,12 @@ static int platform_match(struct device *dev, const struct device_driver *drv) > { > struct platform_device *pdev = to_platform_device(dev); > struct platform_driver *pdrv = to_platform_driver(drv); > + int ret; > > /* When driver_override is set, only bind to the matching driver */ > - if (pdev->driver_override) > - return !strcmp(pdev->driver_override, drv->name); > + ret = device_match_driver_override(dev, drv); > + if (ret >= 0) > + return ret; Same pattern as arch/sh. Consistent refactoring across platform code. > + .driver_override = true, Verify this flag is documented in struct bus_type definition and that all bus_type users expecting driver_override support have this set. > - if (pdev->driver_override) > + if (device_has_driver_override(&pdev->dev)) Good: accessor function prevents direct field access. Confirm device_has_driver_override() is properly exported and inlined where needed for performance. > - ret = driver_set_override(&pdev->dev, &pdev->driver_override, > - "imx-scu-clk", strlen("imx-scu-clk")); > + ret = device_set_driver_override(&pdev->dev, "imx-scu-clk"); Good: removes strlen() call; generic function should handle length internally. Verify no callers relied on old signature. > - pdev_sec->driver_override = kstrdup("samsung-i2s", GFP_KERNEL); > - if (!pdev_sec->driver_override) { > + ret = device_set_driver_override(&pdev_sec->dev, "samsung