diff mbox

[net-next,8/8] net: dsa: mv88e6xxx: fail on mismatching probe

Message ID 20160609004456.5441-9-vivien.didelot@savoirfairelinux.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Vivien Didelot June 9, 2016, 12:44 a.m. UTC
Now that we have access at probe time to the chip info described in the
device tree, check if the probed device matches the device node,
otherwise warn the user and fail.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

Comments

Andrew Lunn June 9, 2016, 2:21 a.m. UTC | #1
On Wed, Jun 08, 2016 at 08:44:56PM -0400, Vivien Didelot wrote:
> Now that we have access at probe time to the chip info described in the
> device tree, check if the probed device matches the device node,
> otherwise warn the user and fail.

What good is this? So what if the device tree says a different
model. We don't care, we don't use that information at all, we read it
from the device itself.

The only thing that might make sense to check is the number of ports
in device tree against what we know the switch has. I don't think we
currently do this. But that actually requires a new method in the
driver structure, so the core can ask the driver after probe how many
ports it has.

       Andrew
Vivien Didelot June 10, 2016, 8:32 p.m. UTC | #2
Hi,

Andrew Lunn <andrew@lunn.ch> writes:

> On Wed, Jun 08, 2016 at 08:44:56PM -0400, Vivien Didelot wrote:
>> Now that we have access at probe time to the chip info described in the
>> device tree, check if the probed device matches the device node,
>> otherwise warn the user and fail.
>
> What good is this? So what if the device tree says a different
> model. We don't care, we don't use that information at all, we read it
> from the device itself.

So we can end up with a badly described device tree. It seems to be a
question of rigor vs. flexibility. I don't know much about the DT
philosophy and I don't really mind as long as we warn the user.

I'd like to have other opinions on this though before pushing v2.

Thanks,

        Vivien
diff mbox

Patch

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 4f07110..8244757 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -21,6 +21,7 @@ 
 #include <linux/list.h>
 #include <linux/mdio.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/of_mdio.h>
 #include <linux/netdevice.h>
 #include <linux/gpio/consumer.h>
@@ -3746,6 +3747,28 @@  static const struct of_device_id mv88e6xxx_of_id_table[] = {
 
 MODULE_DEVICE_TABLE(of, mv88e6xxx_of_id_table);
 
+static bool mv88e6xxx_of_matches(struct mv88e6xxx_priv_state *ps)
+{
+	const struct mv88e6xxx_info *info;
+	const struct of_device_id *id;
+	enum mv88e6xxx_model model;
+
+	id = of_match_device(mv88e6xxx_of_id_table, ps->dev);
+	if (!id)
+		return false;
+
+	model = (enum mv88e6xxx_model)id->data;
+	info = &mv88e6xxx_table[model];
+
+	if (ps->info->prod_num == info->prod_num)
+		return true;
+
+	dev_err(ps->dev, "described node %s mismatches probed model %s\n",
+		id->compatible, ps->info->name);
+
+	return false;
+}
+
 int mv88e6xxx_probe(struct mdio_device *mdiodev)
 {
 	struct device *dev = &mdiodev->dev;
@@ -3758,6 +3781,9 @@  int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (!ps)
 		return -ENODEV;
 
+	if (!mv88e6xxx_of_matches(ps))
+		return -EINVAL;
+
 	ps->reset = devm_gpiod_get(dev, "reset", GPIOD_ASIS);
 	if (IS_ERR(ps->reset)) {
 		err = PTR_ERR(ps->reset);