From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Lunn <andrew@lunn.ch>
Subject: Re: [PATCH 4/4] RFC: net: dsa: realtek-smi: Add Realtek SMI driver
Date: Mon, 6 Nov 2017 00:59:00 +0100
Message-ID: <20171105235900.GB24822@lunn.ch>
References: <20171105231909.5599-1-linus.walleij@linaro.org>
 <20171105231909.5599-5-linus.walleij@linaro.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        netdev@vger.kernel.org,
        Antti =?iso-8859-1?Q?Sepp=E4l=E4?= <a.seppala@gmail.com>,
        Roman Yeryomin <roman@advem.lv>,
        Colin Leitner <colin.leitner@googlemail.com>,
        Gabor Juhos <juhosg@openwrt.org>
To: Linus Walleij <linus.walleij@linaro.org>
Return-path: <netdev-owner@vger.kernel.org>
Received: from vps0.lunn.ch ([185.16.172.187]:43984 "EHLO vps0.lunn.ch"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751246AbdKEX7C (ORCPT <rfc822;netdev@vger.kernel.org>);
        Sun, 5 Nov 2017 18:59:02 -0500
Content-Disposition: inline
In-Reply-To: <20171105231909.5599-5-linus.walleij@linaro.org>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hi Linus

> +static int realtek_smi_read_reg(struct realtek_smi *smi, u32 addr, u32 *data)
> +{
> +	unsigned long flags;
> +	u8 lo = 0;
> +	u8 hi = 0;
> +	int ret;
> +
> +	spin_lock_irqsave(&smi->lock, flags);
> +
> +	realtek_smi_start(smi);
> +
> +	/* send READ command */
> +	ret = realtek_smi_write_byte(smi, smi->cmd_read);
> +	if (ret)
> +		goto out;
> +
> +	/* set ADDR[7:0] */
> +	ret = realtek_smi_write_byte(smi, addr & 0xff);
> +	if (ret)
> +		goto out;
> +
> +	/* set ADDR[15:8] */
> +	ret = realtek_smi_write_byte(smi, addr >> 8);
> +	if (ret)
> +		goto out;
> +
> +	/* read DATA[7:0] */
> +	realtek_smi_read_byte0(smi, &lo);
> +	/* read DATA[15:8] */
> +	realtek_smi_read_byte1(smi, &hi);
> +
> +	*data = ((u32) lo) | (((u32) hi) << 8);

If i'm reading this correct, addr is a u16 and data is also u16?  So
it is pretty similar to SMI.

I'm wondering if this should be modelled as a normal MDIO bus? Put the
driver as drivers/net/mdio-realtek.c?

I need to study the rest of the code to see if this is a good idea or
not.

	Andrew