Hello!

On 7/30/20 2:37 PM, Yoshihiro Shimoda wrote:

quoted

From: Yuusuke Ashizuka, Sent: Thursday, July 30, 2020 7:02 PM
Subject: [PATCH v2] ravb: Fixed the problem that rmmod can not be done

Thank you for the patch! I found a similar patch for another driver [1].

   It's not the same case -- that driver hadn't had the MDIO release code at all
before that patch.

So, we should apply this patch to the ravb driver.

   I believe the driver is innocent. :-)

[1]
fd5f375c1628 ("net-next: ax88796: Attach MII bus only when open")

quoted

ravb is a module driver, but I cannot rmmod it after insmod it.

I think "When this driver is a module, I cannot ..." is better.

   Perhaps "... is built as a module".

quoted

ravb does mdio_init() at the time of probe, and module->refcnt is incremented

I think "This is because that this driver calls ravb_mdio_init() ..." is better.

   Yep.

According to scripts/checkpatch.pl, I think it's better to be a maximum
75 chars per line in the commit description.

   Yes.
   (Note that for the source code the new length limit is 100, not 80.)

quoted

by alloc_mdio_bitbang() called after that.
Therefore, even if ifup is not performed, the driver is in use and rmmod cannot
be performed.

   That's not really obvious...

quoted

$ lsmod
Module                  Size  Used by
ravb                   40960  1
$ rmmod ravb
rmmod: ERROR: Module ravb is in use

   Shouldn't the driver core call the remove() method for the affected devices
first, before checking the refcount? 

quoted

Fixed to execute mdio_init() at open and free_mdio() at close, thereby rmmod is

I think "Fixed to call ravb_mdio_init() at open and ravb_mdio_release() ..." is better.
However, I'm not sure whether that Sergei who is the reviwer of this driver accepts
the descriptions which I suggested though :)

   The language barrier isn't the only obstacle. :-)

By the way, I think you have to send this patch to the following maintainers too:
# We can get it by using scripts/get_maintainers.pl.
David S. Miller [off-list ref] (maintainer:NETWORKING DRIVERS,commit_signer:8/8=100%)
Jakub Kicinski [off-list ref] (maintainer:NETWORKING DRIVERS)

Best regards,
Yoshihiro Shimoda

   For the future, please trim your reply before the patch starts as you
don't comment on the patch itself anyway...

quoted

possible in the ifdown state.

Signed-off-by: Yuusuke Ashizuka <redacted>

[...]

MBR, Sergei