All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yajun Deng <yajun.deng@linux.dev>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, andrew@lunn.ch, olteanv@gmail.com,
	hkallweit1@gmail.com, kabel@kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-phy@lists.infradead.org
Subject: Re: [PATCH net-next] net: phy: Cleanup struct mdio_driver_common
Date: Wed, 3 Jan 2024 10:03:14 +0800	[thread overview]
Message-ID: <a5aca886-ca0a-8170-417f-a189ec28c87f@linux.dev> (raw)
In-Reply-To: <ZZRJLg6U0G5CNRQ0@shell.armlinux.org.uk>


On 2024/1/3 01:34, Russell King (Oracle) wrote:
> On Thu, Dec 28, 2023 at 03:23:50PM +0800, Yajun Deng wrote:
>> The struct mdio_driver_common is a wrapper for driver-model structure,
>> it contains device_driver and flags. There are only struct phy_driver
>> and mdio_driver that use it. The flags is used to distinguish between
>> struct phy_driver and mdio_driver.
>>
>> We can test that if probe of device_driver is equal to phy_probe. This
>> way, the struct mdio_driver_common is no longer needed, and struct
>> phy_driver and usb_mdio_driver will be consistent with other driver
>> structs.
> usb_mdio_driver?

This is a mistake. It should be 'mdio_driver'.

>
> 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 ...


>
>> +bool is_phy_driver(struct device_driver *driver)
>> +{
>> +	return driver->probe == phy_probe;
>> +}
>> +EXPORT_SYMBOL_GPL(is_phy_driver);
> Do we really need this exported? It doesn't seem like something anything
> other than core MDIO/phylib code should know about, and all that becomes
> a single module when building it in a modular way - phylib can't be a
> separate module from mdio stuff.
I think this exported can be removed.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

WARNING: multiple messages have this Message-ID (diff)
From: Yajun Deng <yajun.deng@linux.dev>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, andrew@lunn.ch, olteanv@gmail.com,
	hkallweit1@gmail.com, kabel@kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-phy@lists.infradead.org
Subject: Re: [PATCH net-next] net: phy: Cleanup struct mdio_driver_common
Date: Wed, 3 Jan 2024 10:03:14 +0800	[thread overview]
Message-ID: <a5aca886-ca0a-8170-417f-a189ec28c87f@linux.dev> (raw)
In-Reply-To: <ZZRJLg6U0G5CNRQ0@shell.armlinux.org.uk>


On 2024/1/3 01:34, Russell King (Oracle) wrote:
> On Thu, Dec 28, 2023 at 03:23:50PM +0800, Yajun Deng wrote:
>> The struct mdio_driver_common is a wrapper for driver-model structure,
>> it contains device_driver and flags. There are only struct phy_driver
>> and mdio_driver that use it. The flags is used to distinguish between
>> struct phy_driver and mdio_driver.
>>
>> We can test that if probe of device_driver is equal to phy_probe. This
>> way, the struct mdio_driver_common is no longer needed, and struct
>> phy_driver and usb_mdio_driver will be consistent with other driver
>> structs.
> usb_mdio_driver?

This is a mistake. It should be 'mdio_driver'.

>
> 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 ...


>
>> +bool is_phy_driver(struct device_driver *driver)
>> +{
>> +	return driver->probe == phy_probe;
>> +}
>> +EXPORT_SYMBOL_GPL(is_phy_driver);
> Do we really need this exported? It doesn't seem like something anything
> other than core MDIO/phylib code should know about, and all that becomes
> a single module when building it in a modular way - phylib can't be a
> separate module from mdio stuff.
I think this exported can be removed.

  reply	other threads:[~2024-01-03  2:03 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-28  7:23 [PATCH net-next] net: phy: Cleanup struct mdio_driver_common Yajun Deng
2023-12-28  7:23 ` Yajun Deng
2023-12-28  8:24 ` Przemek Kitszel
2023-12-28  8:24   ` Przemek Kitszel
2023-12-28  8:37   ` Yajun Deng
2023-12-28  8:37     ` Yajun Deng
2024-01-02 17:34 ` Russell King (Oracle)
2024-01-02 17:34   ` Russell King (Oracle)
2024-01-03  2:03   ` Yajun Deng [this message]
2024-01-03  2:03     ` Yajun Deng
2024-01-03 10:51     ` Russell King (Oracle)
2024-01-03 10:51       ` Russell King (Oracle)
2024-01-03 11:38       ` Yajun Deng
2024-01-03 11:38         ` Yajun Deng
2024-01-03 13:30         ` Andrew Lunn
2024-01-03 13:30           ` Andrew Lunn
2024-01-03 18:25 ` kernel test robot
2024-01-03 18:25   ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a5aca886-ca0a-8170-417f-a189ec28c87f@linux.dev \
    --to=yajun.deng@linux.dev \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=kabel@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.