Re: [PATCH v2 net-next v2 09/12] net: dsa: mv88e6xxx: add SMI detection helper

From: Andrew Lunn
Date: Tue Jun 14 2016 - 17:50:41 EST


On Tue, Jun 14, 2016 at 02:31:50PM -0400, Vivien Didelot wrote:
> Extract the allocation and switch ID reading code used by both legacy
> and new probing into an helper function which uses a info structure to
> describe how to access the switch ID register.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@xxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/net/dsa/mv88e6xxx.c | 74 ++++++++++++++++++++-------------------------
> 1 file changed, 32 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index 8ac9f9a..2f36d01 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -3631,22 +3631,15 @@ mv88e6xxx_lookup_info(unsigned int prod_num, const struct mv88e6xxx_info *table,
> return NULL;
> }
>
> -static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
> - struct device *host_dev, int sw_addr,
> - void **priv)
> +static struct mv88e6xxx_priv_state *
> +mv88e6xxx_smi_detect(struct device *dev, struct mii_bus *bus, int sw_addr,
> + const struct mv88e6xxx_info *info)
> {
> - const struct mv88e6xxx_info *info;
> struct mv88e6xxx_priv_state *ps;
> - struct mii_bus *bus;
> - const char *name;
> int id, prod_num, rev;
> - int err;
>
> - bus = dsa_host_dev_to_mii_bus(host_dev);
> - if (!bus)
> - return NULL;
> -
> - id = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), PORT_SWITCH_ID);
> + id = __mv88e6xxx_reg_read(bus, sw_addr, info->port_base_addr,
> + PORT_SWITCH_ID);
> if (id < 0)
> return NULL;
>
> @@ -3658,28 +3651,46 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
> if (!info)
> return NULL;
>
> - name = info->name;
> + dev_info(dev, "switch 0x%x detected: %s, revision %u\n", prod_num,
> + info->name, rev);
>
> - ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
> + ps = devm_kzalloc(dev, sizeof(*ps), GFP_KERNEL);
> if (!ps)
> return NULL;

I don't like the way this detect function goes a lot further than
detection. I would say detection finished when you have the info
structure. Return at that point, and let the probe do the rest.

Andrew