Re: [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq

[luoj]

On 2021-07-29 21:15, Andrew Lunn wrote:
> 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@xxxxxxxxxxxxxx>
> > ---
> >  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.
>
> Hi Andrew
> Thanks for the comments, will update the patch set to keep the name 
> unchanged.
>
> > -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.
>
> will correct it in the next patch set.
>
> > @@ -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.
>
> will keep it unchanged in the next patch set.
>
> > -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

> Hi Andrew, yes, this driver is not compatible with IPQ8064, but it is 
> compatible with
> the new chipset such as ipq807x, ipq60xx and ipq50xx, will take your 
> suggestion in
> the next patch set, thanks for the comments.