diff mbox series

[RFC,net-next,v2,3/9] net: dsa: microchip: perform the compatibility check for dev probed

Message ID 20220513102219.30399-4-arun.ramadoss@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: microchip: refactor the ksz switch init function | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 194 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Arun Ramadoss May 13, 2022, 10:22 a.m. UTC
This patch perform the compatibility check for the device after the chip
detect is done. It is to prevent the mismatch between the device
compatible specified in the device tree and actual device found during
the detect. The ksz9477 device doesn't use any .data in the
of_device_id. But the ksz8795 uses .data for assigning the regmap
between 8830 family and 87xx family switch. Changed the regmap
assignment based on the chip_id from the .data.

Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz8795_spi.c | 37 ++++++++++++++++++++-----
 drivers/net/dsa/microchip/ksz9477_i2c.c | 30 ++++++++++++++++----
 drivers/net/dsa/microchip/ksz9477_spi.c | 30 ++++++++++++++++----
 drivers/net/dsa/microchip/ksz_common.c  | 25 ++++++++++++++++-
 drivers/net/dsa/microchip/ksz_common.h  |  1 +
 5 files changed, 103 insertions(+), 20 deletions(-)

Comments

Vladimir Oltean May 16, 2022, 11:03 a.m. UTC | #1
On Fri, May 13, 2022 at 03:52:13PM +0530, Arun Ramadoss wrote:
> This patch perform the compatibility check for the device after the chip
> detect is done. It is to prevent the mismatch between the device
> compatible specified in the device tree and actual device found during
> the detect. The ksz9477 device doesn't use any .data in the
> of_device_id. But the ksz8795 uses .data for assigning the regmap
> between 8830 family and 87xx family switch. Changed the regmap
> assignment based on the chip_id from the .data.
> 
> Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
> ---
>  drivers/net/dsa/microchip/ksz8795_spi.c | 37 ++++++++++++++++++++-----
>  drivers/net/dsa/microchip/ksz9477_i2c.c | 30 ++++++++++++++++----
>  drivers/net/dsa/microchip/ksz9477_spi.c | 30 ++++++++++++++++----
>  drivers/net/dsa/microchip/ksz_common.c  | 25 ++++++++++++++++-
>  drivers/net/dsa/microchip/ksz_common.h  |  1 +
>  5 files changed, 103 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz8795_spi.c b/drivers/net/dsa/microchip/ksz8795_spi.c
> index 5f8d94aee774..1ae1b1ee9f2a 100644
> --- a/drivers/net/dsa/microchip/ksz8795_spi.c
> +++ b/drivers/net/dsa/microchip/ksz8795_spi.c
> @@ -31,9 +31,12 @@ KSZ_REGMAP_TABLE(ksz8795, 16, KSZ8795_SPI_ADDR_SHIFT,
>  KSZ_REGMAP_TABLE(ksz8863, 16, KSZ8863_SPI_ADDR_SHIFT,
>  		 KSZ8863_SPI_TURNAROUND_SHIFT, KSZ8863_SPI_ADDR_ALIGN);
>  
> +#define KSZ_88X3_FAMILY 0x8830
> +

Can we have this macro defined in ksz_common.h and used in the chip_id
of the ksz_chip_data structure as well? It makes things easier to follow
by pattern matching. And for symmetry, it would probably be good to have
such a macro for all chip ids.
Vladimir Oltean May 16, 2022, 11:08 a.m. UTC | #2
On Fri, May 13, 2022 at 03:52:13PM +0530, Arun Ramadoss wrote:
> +static int ksz_check_device_id(struct ksz_device *dev)
> +{
> +	const struct ksz_chip_data *dt_chip_data;
> +
> +	dt_chip_data = of_device_get_match_data(dev->dev);

And one other comment. You haven't converted ksz8863_smi.c to put
anything in struct of_device_id :: data, so that driver will dereference
NULL here.

> +
> +	/* Check for Device Tree and Chip ID */
> +	if (dt_chip_data->chip_id != dev->chip_id) {
> +		dev_err(dev->dev,
> +			"Device tree specifies chip %s but found %s, please fix it!\n",
> +			dt_chip_data->dev_name, dev->info->dev_name);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz8795_spi.c b/drivers/net/dsa/microchip/ksz8795_spi.c
index 5f8d94aee774..1ae1b1ee9f2a 100644
--- a/drivers/net/dsa/microchip/ksz8795_spi.c
+++ b/drivers/net/dsa/microchip/ksz8795_spi.c
@@ -31,9 +31,12 @@  KSZ_REGMAP_TABLE(ksz8795, 16, KSZ8795_SPI_ADDR_SHIFT,
 KSZ_REGMAP_TABLE(ksz8863, 16, KSZ8863_SPI_ADDR_SHIFT,
 		 KSZ8863_SPI_TURNAROUND_SHIFT, KSZ8863_SPI_ADDR_ALIGN);
 
+#define KSZ_88X3_FAMILY 0x8830
+
 static int ksz8795_spi_probe(struct spi_device *spi)
 {
 	const struct regmap_config *regmap_config;
+	const struct ksz_chip_data *chip;
 	struct device *ddev = &spi->dev;
 	struct regmap_config rc;
 	struct ksz_device *dev;
@@ -50,10 +53,15 @@  static int ksz8795_spi_probe(struct spi_device *spi)
 	if (!dev)
 		return -ENOMEM;
 
-	regmap_config = device_get_match_data(ddev);
-	if (!regmap_config)
+	chip = device_get_match_data(ddev);
+	if (!chip)
 		return -EINVAL;
 
+	if (chip->chip_id == KSZ_88X3_FAMILY)
+		regmap_config = ksz8863_regmap_config;
+	else
+		regmap_config = ksz8795_regmap_config;
+
 	for (i = 0; i < ARRAY_SIZE(ksz8795_regmap_config); i++) {
 		rc = regmap_config[i];
 		rc.lock_arg = &dev->regmap_mutex;
@@ -113,11 +121,26 @@  static void ksz8795_spi_shutdown(struct spi_device *spi)
 }
 
 static const struct of_device_id ksz8795_dt_ids[] = {
-	{ .compatible = "microchip,ksz8765", .data = &ksz8795_regmap_config },
-	{ .compatible = "microchip,ksz8794", .data = &ksz8795_regmap_config },
-	{ .compatible = "microchip,ksz8795", .data = &ksz8795_regmap_config },
-	{ .compatible = "microchip,ksz8863", .data = &ksz8863_regmap_config },
-	{ .compatible = "microchip,ksz8873", .data = &ksz8863_regmap_config },
+	{
+		.compatible = "microchip,ksz8765",
+		.data = &ksz_switch_chips[KSZ8765]
+	},
+	{
+		.compatible = "microchip,ksz8794",
+		.data = &ksz_switch_chips[KSZ8794]
+	},
+	{
+		.compatible = "microchip,ksz8795",
+		.data = &ksz_switch_chips[KSZ8795]
+	},
+	{
+		.compatible = "microchip,ksz8863",
+		.data = &ksz_switch_chips[KSZ8830]
+	},
+	{
+		.compatible = "microchip,ksz8873",
+		.data = &ksz_switch_chips[KSZ8830]
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, ksz8795_dt_ids);
diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c
index cbc0b20e7e1b..faa3163c86b0 100644
--- a/drivers/net/dsa/microchip/ksz9477_i2c.c
+++ b/drivers/net/dsa/microchip/ksz9477_i2c.c
@@ -87,12 +87,30 @@  static const struct i2c_device_id ksz9477_i2c_id[] = {
 MODULE_DEVICE_TABLE(i2c, ksz9477_i2c_id);
 
 static const struct of_device_id ksz9477_dt_ids[] = {
-	{ .compatible = "microchip,ksz9477" },
-	{ .compatible = "microchip,ksz9897" },
-	{ .compatible = "microchip,ksz9893" },
-	{ .compatible = "microchip,ksz9563" },
-	{ .compatible = "microchip,ksz9567" },
-	{ .compatible = "microchip,ksz8563" },
+	{
+		.compatible = "microchip,ksz9477",
+		.data = &ksz_switch_chips[KSZ9477]
+	},
+	{
+		.compatible = "microchip,ksz9897",
+		.data = &ksz_switch_chips[KSZ9897]
+	},
+	{
+		.compatible = "microchip,ksz9893",
+		.data = &ksz_switch_chips[KSZ9893]
+	},
+	{
+		.compatible = "microchip,ksz9563",
+		.data = &ksz_switch_chips[KSZ9893]
+	},
+	{
+		.compatible = "microchip,ksz8563",
+		.data = &ksz_switch_chips[KSZ9893]
+	},
+	{
+		.compatible = "microchip,ksz9567",
+		.data = &ksz_switch_chips[KSZ9567]
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, ksz9477_dt_ids);
diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index 87ca464dad32..1bc8b0cbe458 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -86,12 +86,30 @@  static void ksz9477_spi_shutdown(struct spi_device *spi)
 }
 
 static const struct of_device_id ksz9477_dt_ids[] = {
-	{ .compatible = "microchip,ksz9477" },
-	{ .compatible = "microchip,ksz9897" },
-	{ .compatible = "microchip,ksz9893" },
-	{ .compatible = "microchip,ksz9563" },
-	{ .compatible = "microchip,ksz8563" },
-	{ .compatible = "microchip,ksz9567" },
+	{
+		.compatible = "microchip,ksz9477",
+		.data = &ksz_switch_chips[KSZ9477]
+	},
+	{
+		.compatible = "microchip,ksz9897",
+		.data = &ksz_switch_chips[KSZ9897]
+	},
+	{
+		.compatible = "microchip,ksz9893",
+		.data = &ksz_switch_chips[KSZ9893]
+	},
+	{
+		.compatible = "microchip,ksz9563",
+		.data = &ksz_switch_chips[KSZ9893]
+	},
+	{
+		.compatible = "microchip,ksz8563",
+		.data = &ksz_switch_chips[KSZ9893]
+	},
+	{
+		.compatible = "microchip,ksz9567",
+		.data = &ksz_switch_chips[KSZ9567]
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, ksz9477_dt_ids);
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index f556647d2b61..fd2f1bd3feb5 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -14,6 +14,7 @@ 
 #include <linux/phy.h>
 #include <linux/etherdevice.h>
 #include <linux/if_bridge.h>
+#include <linux/of_device.h>
 #include <linux/of_net.h>
 #include <net/dsa.h>
 #include <net/switchdev.h>
@@ -59,7 +60,7 @@  struct ksz_stats_raw {
 	u64 tx_discards;
 };
 
-static const struct ksz_chip_data ksz_switch_chips[] = {
+const struct ksz_chip_data ksz_switch_chips[] = {
 	[KSZ8795] = {
 		.chip_id = 0x8795,
 		.dev_name = "KSZ8795",
@@ -210,6 +211,7 @@  static const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_cnt = 8,		/* total physical port count */
 	},
 };
+EXPORT_SYMBOL_GPL(ksz_switch_chips);
 
 static const struct ksz_chip_data *ksz_lookup_info(unsigned int prod_num)
 {
@@ -225,6 +227,23 @@  static const struct ksz_chip_data *ksz_lookup_info(unsigned int prod_num)
 	return NULL;
 }
 
+static int ksz_check_device_id(struct ksz_device *dev)
+{
+	const struct ksz_chip_data *dt_chip_data;
+
+	dt_chip_data = of_device_get_match_data(dev->dev);
+
+	/* Check for Device Tree and Chip ID */
+	if (dt_chip_data->chip_id != dev->chip_id) {
+		dev_err(dev->dev,
+			"Device tree specifies chip %s but found %s, please fix it!\n",
+			dt_chip_data->dev_name, dev->info->dev_name);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 void ksz_r_mib_stats64(struct ksz_device *dev, int port)
 {
 	struct rtnl_link_stats64 *stats;
@@ -741,6 +760,10 @@  int ksz_switch_register(struct ksz_device *dev,
 	/* Update the compatible info with the probed one */
 	dev->info = info;
 
+	ret = ksz_check_device_id(dev);
+	if (ret)
+		return ret;
+
 	ret = dev->dev_ops->init(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 7d87738329de..d6c4c4b7f7bf 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -177,6 +177,7 @@  void ksz_init_mib_timer(struct ksz_device *dev);
 void ksz_r_mib_stats64(struct ksz_device *dev, int port);
 void ksz_get_stats64(struct dsa_switch *ds, int port,
 		     struct rtnl_link_stats64 *s);
+extern const struct ksz_chip_data ksz_switch_chips[];
 
 /* Common DSA access functions */