Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Luo Jie <luoj@codeaurora.org>
Cc: hkallweit1@gmail.com, davem@davemloft.net, kuba@kernel.org,
	p.zabel@pengutronix.de, agross@kernel.org,
	bjorn.andersson@linaro.org, robh+dt@kernel.org,
	robert.marko@sartura.hr, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, sricharan@codeaurora.org
Subject: Re: [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq
Date: Thu, 29 Jul 2021 15:15:34 +0200	[thread overview]
Message-ID: <YQKp9gsnjBNmXYIc@lunn.ch> (raw)
In-Reply-To: <20210729125358.5227-2-luoj@codeaurora.org>

On Thu, Jul 29, 2021 at 08:53:57PM +0800, Luo Jie wrote:
> mdio_ipq driver supports more SOCs such as ipq40xx, ipq807x,
> ipq60xx and ipq50xx.
> 
> Signed-off-by: Luo Jie <luoj@codeaurora.org>
> ---
>  drivers/net/mdio/Kconfig                      |  6 +-
>  drivers/net/mdio/Makefile                     |  2 +-
>  .../net/mdio/{mdio-ipq4019.c => mdio-ipq.c}   | 66 +++++++++----------
>  3 files changed, 37 insertions(+), 37 deletions(-)
>  rename drivers/net/mdio/{mdio-ipq4019.c => mdio-ipq.c} (81%)

Hi Luo

We don't rename files unless there is a very good reason. It makes
back porting of fixes harder in stable. There are plenty of examples
of files with device specific names, but supporting a broad range of
devices. Take for example lm75, at24.

> -config MDIO_IPQ4019
> -	tristate "Qualcomm IPQ4019 MDIO interface support"
> +config MDIO_IPQ
> +	tristate "Qualcomm IPQ MDIO interface support"
>  	depends on HAS_IOMEM && OF_MDIO
>  	depends on GPIOLIB && COMMON_CLK && RESET_CONTROLLER
>  	help
>  	  This driver supports the MDIO interface found in Qualcomm
> -	  IPQ40xx series Soc-s.
> +	  IPQ40xx, IPQ60XX, IPQ807X and IPQ50XX series Soc-s.

Please leave the MDIO_IPQ4019 unchanged, so we don't break backwards
compatibility, but the changes to the text are O.K.

> @@ -31,38 +31,38 @@
>  /* 0 = Clause 22, 1 = Clause 45 */
>  #define MDIO_MODE_C45				BIT(8)
>  
> -#define IPQ4019_MDIO_TIMEOUT	10000
> -#define IPQ4019_MDIO_SLEEP		10
> +#define IPQ_MDIO_TIMEOUT	10000
> +#define IPQ_MDIO_SLEEP		10

This sort of mass rename will also make back porting fixes
harder. Please don't do it.

> -static const struct of_device_id ipq4019_mdio_dt_ids[] = {
> +static const struct of_device_id ipq_mdio_dt_ids[] = {
>  	{ .compatible = "qcom,ipq4019-mdio" },
> +	{ .compatible = "qcom,ipq-mdio" },
>  	{ }
>  };

Such a generic name is not a good idea. It appears this driver is not
compatible with the IPQ8064? It is O.K. to add more specific
compatibles. So you could add

qcom,ipq40xx, qcom,ipq60xx, qcom,ipq807x and qcom,ipq50xx.

But really, there is no need. Take for example snps,dwmac-mdio, which
is used in all sorts of devices.

   Andrew

  reply	other threads:[~2021-07-29 13:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29 12:53 [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Luo Jie
2021-07-29 12:53 ` [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq Luo Jie
2021-07-29 13:15   ` Andrew Lunn [this message]
2021-08-02  5:56     ` luoj
2021-07-29 12:53 ` [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings Luo Jie
2021-07-29 13:17   ` Andrew Lunn
2021-08-02  6:02     ` luoj
2021-07-29 13:57   ` Rob Herring
2021-07-29 17:29   ` Rob Herring
2021-08-02  7:19     ` luoj
2021-08-02 13:39       ` Andrew Lunn
2021-08-04  2:37         ` Jie Luo
2021-07-29 13:26 ` [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Andrew Lunn
2021-08-02  6:46   ` luoj

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=YQKp9gsnjBNmXYIc@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luoj@codeaurora.org \
    --cc=netdev@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=robert.marko@sartura.hr \
    --cc=robh+dt@kernel.org \
    --cc=sricharan@codeaurora.org \
    --subject='Re: [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq' \
    /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

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox