Re: [PATCH 2/2] soc: bcm: iproc: Add Broadcom iProc IDM driver

From: Ray Jui
Date: Mon Dec 09 2019 - 13:05:17 EST




On 12/7/19 9:52 AM, Florian Fainelli wrote:


On 12/6/2019 5:15 PM, Ray Jui wrote:

Did not you intend to drop the reference count on elog_np here?


Sorry, I'm not following this comment. Could you please help to clarify
for me a bit more? Thanks!

I meant that you drop the reference count on 'np' but you called
functions that incremented the reference count on 'elog_np', so maybe
you are not doing the of_node_put() on the appropriate device_node
reference?


Okay thanks. I'll look into this in more details and make corrections if needed.


[snip]

+static struct platform_driver iproc_idm_driver = {
+ÂÂÂ .probe = iproc_idm_probe,

Do not you need a remove function in order to unregister the sysfs file
that you created in iproc_idm_dev_probe() to avoid bind/unbind (or
rmmod/modprobe) to spit out an existing sysfs entry warning?


This driver should never be compiled as a module. It's meant to be
always there to capture IDM bus timeouts.

But you are right that I cannot prevent user from trying to unbind it
for whatever reason. I'll add a remove routine to take care of this.

You can also set suppress_bind_attrs to your platform_driver structure
to prevent unbind/bind from happening.


Great. This is what I'll do then. I meant to have this driver stays loaded/binded all the time once probed.

Thanks,

Ray