Patchwork [RFD] mv88e6060: Allow the driver to be probed from device tree

login
register
mail settings
Submitter Pavel Machek
Date Dec. 6, 2018, 1:02 p.m.
Message ID <20181206130246.GC22343@amd>
Download mbox | patch
Permalink /patch/674105/
State New
Headers show

Comments

Pavel Machek - Dec. 6, 2018, 1:02 p.m.
mv88e6060: Allow the driver to be probed from device tree
    
This is NOT ready for merge. Hardcoding an address is a bad idea, and obviously
it would be good to allow module removal, too.
    
Signed-off-by: Pavel Machek <pavel@ucw.cz> (not ready)
kbuild test robot - Dec. 6, 2018, 6:32 p.m.
Hi Pavel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.20-rc5 next-20181206]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Pavel-Machek/mv88e6060-Allow-the-driver-to-be-probed-from-device-tree/20181207-013430
config: i386-randconfig-x007-201848 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/net/dsa/mv88e6060.c: In function 'mv88e6060_probe':
>> drivers/net/dsa/mv88e6060.c:316:16: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      char *name = mv88e6060_get_name(mdiodev->bus, addr);
                   ^~~~~~~~~~~~~~~~~~
>> drivers/net/dsa/mv88e6060.c:317:34: warning: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'struct mii_bus *' [-Wformat=]
      printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
                                   ~~^               ~~~~~~~~~~~~
>> drivers/net/dsa/mv88e6060.c:317:38: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'int' [-Wformat=]
      printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
                                       ~~^
                                       %x
   drivers/net/dsa/mv88e6060.c:307:6: warning: unused variable 'err' [-Wunused-variable]
     int err;
         ^~~
   drivers/net/dsa/mv88e6060.c:306:6: warning: unused variable 'eeprom_len' [-Wunused-variable]
     u32 eeprom_len;
         ^~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:304:31: warning: unused variable 'compat_info' [-Wunused-variable]
     const struct mv88e6060_info *compat_info;
                                  ^~~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:303:22: warning: unused variable 'np' [-Wunused-variable]
     struct device_node *np = dev->of_node;
                         ^~
   drivers/net/dsa/mv88e6060.c: In function 'mv88e6060_remove':
   drivers/net/dsa/mv88e6060.c:344:25: warning: unused variable 'chip' [-Wunused-variable]
     struct mv88e6060_chip *chip = ds->priv;
                            ^~~~
   Cyclomatic Complexity 1 include/linux/device.h:devm_kzalloc
   Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
   Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata
   Cyclomatic Complexity 1 include/linux/etherdevice.h:eth_random_addr
   Cyclomatic Complexity 1 include/net/dsa.h:dsa_to_port
   Cyclomatic Complexity 1 include/net/dsa.h:dsa_is_cpu_port
   Cyclomatic Complexity 1 include/net/dsa.h:dsa_is_user_port
   Cyclomatic Complexity 3 include/net/dsa.h:dsa_user_ports
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:mv88e6060_get_tag_protocol
   Cyclomatic Complexity 2 drivers/net/dsa/mv88e6060.c:alloc_priv
   Cyclomatic Complexity 2 drivers/net/dsa/mv88e6060.c:mv88e6060_port_to_phy_addr
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:mv88e6060_init
   Cyclomatic Complexity 0 drivers/net/dsa/mv88e6060.c:mv88e6060_remove
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:reg_read
   Cyclomatic Complexity 2 drivers/net/dsa/mv88e6060.c:mv88e6060_phy_read
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:reg_write
   Cyclomatic Complexity 2 drivers/net/dsa/mv88e6060.c:mv88e6060_phy_write
   Cyclomatic Complexity 3 drivers/net/dsa/mv88e6060.c:mv88e6060_setup_global
   Cyclomatic Complexity 4 drivers/net/dsa/mv88e6060.c:mv88e6060_setup_addr
   Cyclomatic Complexity 6 drivers/net/dsa/mv88e6060.c:mv88e6060_setup_port
   Cyclomatic Complexity 13 drivers/net/dsa/mv88e6060.c:mv88e6060_switch_reset
   Cyclomatic Complexity 6 drivers/net/dsa/mv88e6060.c:mv88e6060_setup
   Cyclomatic Complexity 5 drivers/net/dsa/mv88e6060.c:mv88e6060_get_name
   Cyclomatic Complexity 3 drivers/net/dsa/mv88e6060.c:mv88e6060_probe
   Cyclomatic Complexity 3 drivers/net/dsa/mv88e6060.c:mv88e6060_drv_probe
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:mv88e6060_cleanup
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:_GLOBAL__sub_I_00100_0_mv88e6060.c
   Cyclomatic Complexity 1 drivers/net/dsa/mv88e6060.c:_GLOBAL__sub_D_00100_1_mv88e6060.c

vim +/const +316 drivers/net/dsa/mv88e6060.c

   299	
   300	static int mv88e6060_probe(struct mdio_device *mdiodev)
   301	{
   302		struct device *dev = &mdiodev->dev;
   303		struct device_node *np = dev->of_node;
   304		const struct mv88e6060_info *compat_info;
   305		struct mv88e6060_priv *chip;
   306		u32 eeprom_len;
   307		int err;
   308	
   309		int addr = 0x10 /* mdiodev->addr */ ;
   310	
   311		chip = alloc_priv(dev, mdiodev->bus, addr);
   312		if (!chip)
   313			return -ENOMEM;
   314	
   315		{
 > 316			char *name = mv88e6060_get_name(mdiodev->bus, addr);
 > 317			printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
   318		}
   319		{
   320		  	struct dsa_switch *ds;
   321	
   322			ds = dsa_switch_alloc(dev, 6);
   323			if (!ds)
   324				return -ENOMEM;
   325	
   326			ds->priv = chip;
   327			ds->dev = dev;
   328			ds->ops = &mv88e6060_switch_ops;
   329			ds->ageing_time_min = 15000;
   330			ds->ageing_time_max = 15000 * U8_MAX;
   331	
   332			dev_set_drvdata(dev, ds);
   333	
   334			return dsa_register_switch(ds);
   335		}
   336	
   337		printk("e6060: probe ok\n");
   338		return 0;
   339	}
   340	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kbuild test robot - Dec. 15, 2018, 8:41 a.m.
Hi Pavel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.20-rc6 next-20181214]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Pavel-Machek/mv88e6060-Allow-the-driver-to-be-probed-from-device-tree/20181207-013430
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

>> drivers/net/dsa/mv88e6060.c:316:48: warning: incorrect type in initializer (different modifiers)
   drivers/net/dsa/mv88e6060.c:316:48:    expected char *name
   drivers/net/dsa/mv88e6060.c:316:48:    got char const *
   drivers/net/dsa/mv88e6060.c: In function 'mv88e6060_probe':
   drivers/net/dsa/mv88e6060.c:316:16: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      char *name = mv88e6060_get_name(mdiodev->bus, addr);
                   ^~~~~~~~~~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:317:34: warning: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'struct mii_bus *' [-Wformat=]
      printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
                                   ~~^               ~~~~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:317:38: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'int' [-Wformat=]
      printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
                                       ~~^
                                       %x
   drivers/net/dsa/mv88e6060.c:307:6: warning: unused variable 'err' [-Wunused-variable]
     int err;
         ^~~
   drivers/net/dsa/mv88e6060.c:306:6: warning: unused variable 'eeprom_len' [-Wunused-variable]
     u32 eeprom_len;
         ^~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:304:31: warning: unused variable 'compat_info' [-Wunused-variable]
     const struct mv88e6060_info *compat_info;
                                  ^~~~~~~~~~~
   drivers/net/dsa/mv88e6060.c:303:22: warning: unused variable 'np' [-Wunused-variable]
     struct device_node *np = dev->of_node;
                         ^~
   drivers/net/dsa/mv88e6060.c: In function 'mv88e6060_remove':
   drivers/net/dsa/mv88e6060.c:344:25: warning: unused variable 'chip' [-Wunused-variable]
     struct mv88e6060_chip *chip = ds->priv;
                            ^~~~

vim +316 drivers/net/dsa/mv88e6060.c

   299	
   300	static int mv88e6060_probe(struct mdio_device *mdiodev)
   301	{
   302		struct device *dev = &mdiodev->dev;
   303		struct device_node *np = dev->of_node;
   304		const struct mv88e6060_info *compat_info;
   305		struct mv88e6060_priv *chip;
   306		u32 eeprom_len;
   307		int err;
   308	
   309		int addr = 0x10 /* mdiodev->addr */ ;
   310	
   311		chip = alloc_priv(dev, mdiodev->bus, addr);
   312		if (!chip)
   313			return -ENOMEM;
   314	
   315		{
 > 316			char *name = mv88e6060_get_name(mdiodev->bus, addr);
   317			printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
   318		}
   319		{
   320		  	struct dsa_switch *ds;
   321	
   322			ds = dsa_switch_alloc(dev, 6);
   323			if (!ds)
   324				return -ENOMEM;
   325	
   326			ds->priv = chip;
   327			ds->dev = dev;
   328			ds->ops = &mv88e6060_switch_ops;
   329			ds->ageing_time_min = 15000;
   330			ds->ageing_time_max = 15000 * U8_MAX;
   331	
   332			dev_set_drvdata(dev, ds);
   333	
   334			return dsa_register_switch(ds);
   335		}
   336	
   337		printk("e6060: probe ok\n");
   338		return 0;
   339	}
   340	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch

diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index f43104f48dbb..9e3901f18518 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -77,26 +77,40 @@  static const char *mv88e6060_get_name(struct mii_bus *bus, int sw_addr)
 static enum dsa_tag_protocol mv88e6060_get_tag_protocol(struct dsa_switch *ds,
 							int port)
 {
+  //return DSA_TAG_PROTO_QCA;
+  //return DSA_TAG_PROTO_TRAILER;
 	return DSA_TAG_PROTO_TRAILER;
 }
 
+static struct mv88e6060_priv *
+alloc_priv(struct device *dsa_dev, struct mii_bus *bus, int sw_addr)
+{
+  	struct mv88e6060_priv *priv;
+
+	priv = devm_kzalloc(dsa_dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return NULL;
+
+	priv->bus = bus;
+	priv->sw_addr = sw_addr;
+	return priv;
+}
+
 static const char *mv88e6060_drv_probe(struct device *dsa_dev,
 				       struct device *host_dev, int sw_addr,
 				       void **_priv)
 {
 	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
-	struct mv88e6060_priv *priv;
 	const char *name;
 
 	name = mv88e6060_get_name(bus, sw_addr);
-	if (name) {
-		priv = devm_kzalloc(dsa_dev, sizeof(*priv), GFP_KERNEL);
-		if (!priv)
-			return NULL;
-		*_priv = priv;
-		priv->bus = bus;
-		priv->sw_addr = sw_addr;
-	}
+	
+	if (!name)
+		return NULL;
+
+	*_priv = alloc_priv(dsa_dev, bus, sw_addr);
+	if (!*_priv)
+		  return NULL;
 
 	return name;
 }
@@ -132,9 +146,12 @@  static int mv88e6060_switch_reset(struct dsa_switch *ds)
 
 		usleep_range(1000, 2000);
 	}
-	if (time_after(jiffies, timeout))
+	if (time_after(jiffies, timeout)) {
+		printk("e6060: reset timed out!\n");
 		return -ETIMEDOUT;
+	}
 
+	printk("e6060: reset ok\n");	
 	return 0;
 }
 
@@ -284,15 +301,94 @@  static struct dsa_switch_driver mv88e6060_switch_drv = {
 	.ops		= &mv88e6060_switch_ops,
 };
 
+static int mv88e6060_probe(struct mdio_device *mdiodev)
+{
+	struct device *dev = &mdiodev->dev;
+	struct device_node *np = dev->of_node;
+	const struct mv88e6060_info *compat_info;
+	struct mv88e6060_priv *chip;
+	u32 eeprom_len;
+	int err;
+
+	int addr = 0x10 /* mdiodev->addr */ ;
+
+	chip = alloc_priv(dev, mdiodev->bus, addr);
+	if (!chip)
+		return -ENOMEM;
+
+	{
+		char *name = mv88e6060_get_name(mdiodev->bus, addr);
+		printk("e6060: got name %s @ %lx %lx\n", name, mdiodev->bus, addr);
+	}
+	{
+	  	struct dsa_switch *ds;
+
+		ds = dsa_switch_alloc(dev, 6);
+		if (!ds)
+			return -ENOMEM;
+
+		ds->priv = chip;
+		ds->dev = dev;
+		ds->ops = &mv88e6060_switch_ops;
+		ds->ageing_time_min = 15000;
+		ds->ageing_time_max = 15000 * U8_MAX;
+
+		dev_set_drvdata(dev, ds);
+
+		return dsa_register_switch(ds);
+	}
+
+	printk("e6060: probe ok\n");
+	return 0;
+}
+
+static void mv88e6060_remove(struct mdio_device *mdiodev)
+{
+	struct dsa_switch *ds = dev_get_drvdata(&mdiodev->dev);
+	struct mv88e6060_chip *chip = ds->priv;
+
+	printk("e6060: remove.\n");
+	BUG();
+#if 0
+	mv88e6060_phy_destroy(chip);
+	mv88e6060_unregister_switch(chip);
+	mv88e6060_mdio_unregister(chip);
+#endif
+}
+
+static const struct of_device_id mv88e6060_of_match[] = {
+	{
+		.compatible = "marvell,mv88e6060",
+	},
+	{ /* sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, mv88e6060_of_match);
+
+static struct mdio_driver mv88e6060_driver = {
+	.probe	= mv88e6060_probe,
+	.remove = mv88e6060_remove,
+	.mdiodrv.driver = {
+		.name = "mv88e6060",
+		.of_match_table = mv88e6060_of_match,
+	},
+};
+
 static int __init mv88e6060_init(void)
 {
+	int r;
+	printk("e6060: init\n");
 	register_switch_driver(&mv88e6060_switch_drv);
-	return 0;
+	r = mdio_driver_register(&mv88e6060_driver);
+	printk("e6060: init %d\n", r);
+	return r;
 }
+
 module_init(mv88e6060_init);
 
 static void __exit mv88e6060_cleanup(void)
 {
+  	mdio_driver_unregister(&mv88e6060_driver);
 	unregister_switch_driver(&mv88e6060_switch_drv);
 }
 module_exit(mv88e6060_cleanup);