On 07/14/2015 01:27 AM, Bjorn Andersson wrote: > On Tue 07 Jul 06:45 PDT 2015, Georgi Djakov wrote: > >> Hi Bjorn, >> Thank you for this patchset! Some nits and a question below. >> > > Thank you! > >> On 06/27/2015 12:50 AM, [email protected] wrote: >>> From: Bjorn Andersson <[email protected]> >>> >>> This adds the Qualcomm Shared Memory Driver (SMD) providing >>> communication channels to remote processors, ontop of SMEM. >>> >>> Signed-off-by: Bjorn Andersson <[email protected]> [..] >> [...] >>> +static int __init qcom_smd_init(void) >>> +{ >>> + int ret; >>> + >>> + ret = bus_register(&qcom_smd_bus); >>> + if (ret) { >>> + pr_err("failed to register smd bus: %d\n", ret); >>> + return ret; >>> + } >>> + >>> + return platform_driver_register(&qcom_smd_driver); >>> +} >>> +arch_initcall(qcom_smd_init); >>> + >>> +static void __exit qcom_smd_exit(void) >>> +{ >>> + platform_driver_unregister(&qcom_smd_driver); >>> + bus_unregister(&qcom_smd_bus); >>> +} >>> +module_exit(qcom_smd_exit); >>> + >> [...] >>> +/** >>> + * struct qcom_smd_driver - smd driver struct >>> + * @driver: underlying device driver >>> + * @probe: invoked when the smd channel is found >>> + * @remove: invoked when the smd channel is closed >>> + * @callback: invoked when an inbound message is received on the >>> channel, >>> + * should return 0 on success or -EBUSY if the data cannot be >>> + * consumed at this time >>> + */ >>> +struct qcom_smd_driver { >>> + struct device_driver driver; >>> + int (*probe)(struct qcom_smd_device *dev); >>> + void (*remove)(struct qcom_smd_device *dev); >>> + int (*callback)(struct qcom_smd_device *, const void *, size_t); >>> +}; >>> + >>> +int qcom_smd_driver_register(struct qcom_smd_driver *drv); >>> +void qcom_smd_driver_unregister(struct qcom_smd_driver *drv); >>> + >>> +#define module_qcom_smd_driver(__smd_driver) \ >>> + module_driver(__smd_driver, qcom_smd_driver_register, \ >>> + qcom_smd_driver_unregister) >>> + >> >> This comment is mostly related to your RPM over SMD driver patch, as >> i have a RPM clock driver based on it. The RPM clock driver registers >> some fundamental stuff like XO and i had to hack smd-rpm to probe >> earlier, so that most other drivers can initialize. So i was wondering, >> what if we register the drivers on the bus earlier? What do you think? >> > > My only concern would be that if we're calling > qcom_smd_driver_register() before the smd arch_initcall has registered > the bus it will fail. >
Maybe we can use core/postcore_initcall for smd, and then arch_initcall for smd-rpm? > Part of this I see no problem with modifying the rpm driver to register > earlier - and it would be good to have those regulators earlier as > well... Booting with initcall_debug shows me that most busses are registered at postcore_initcall - like spmi, i2c, spi etc. > I've intentionally not done anything about this, because it's helped to > smoke out a bunch of EPROBE_DEFER issues for me already, but longer term > it's not okay for all our drivers to fail 2-3 times before the > regulators are up... I agree. Thanks! BR, Georgi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
