On Wed, Jan 03, 2024 at 07:38:04PM +0800, Yajun Deng wrote:

On 2024/1/3 18:51, Russell King (Oracle) wrote:

quoted

On Wed, Jan 03, 2024 at 10:03:14AM +0800, Yajun Deng wrote:

quoted

On 2024/1/3 01:34, Russell King (Oracle) wrote:

quoted

I'm not sure why this consistency is even desired, the commit message
doesn't properly say _why_ this change is being proposed.

Most drivers use device_driver directly. This should be added to the commit.

Like this:

struct sdio_driver {

... ...

         struct device_driver drv;
};


struct pcie_port_service_driver {

... ...

         struct device_driver driver;
};

and so on ...

... which is fine for those other drivers because they don't share the
same bus. That is not the case here - we have two different classes
of drivers on the same bus.

Yes, that's true. But we can implement it with is_phy_driver(). I don't
think we need a flag for that.

quoted

I don't like a justification that just because other subsystems do
something in one particular way, that is the only way things should be
done. I think there is good reason to have the structure we have, and
thus there needs to be a good reason to change it.

Its purpose is to clean up struct mdio_driver_common, and make the code
cleaner.

I have to agree with Russell here. The commit message should explain
the 'Why?'. Why is this better? Why is it cleaner? Why does making it
the same as all other drivers make it better, when in fact we have two
classes of devices stacked on top of it, and making it different to
every other driver actually helps developers realise that?

      Andrew