Subject: Re: [PATCH] net: stmmac: Add OXNAS Glue Driver
To: Rob Herring <robh@kernel.org>
References: <20161021084445.24989-1-narmstrong@baylibre.com>
 <20161030204110.otvjlanmi7dv7fox@rob-hp-laptop>
Cc: peppe.cavallaro@st.com, alexandre.torgue@st.com,
        netdev@vger.kernel.org, linux-oxnas@lists.tuxfamily.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org
From: Neil Armstrong <narmstrong@baylibre.com>
Organization: Baylibre
Message-ID: <04b8ed79-8efa-cc10-a9a3-fdee10e0723c@baylibre.com>
Date: Mon, 31 Oct 2016 10:55:39 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.3.0
MIME-Version: 1.0
In-Reply-To: <20161030204110.otvjlanmi7dv7fox@rob-hp-laptop>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3078
Lines: 90

On 10/30/2016 09:41 PM, Rob Herring wrote:
> On Fri, Oct 21, 2016 at 10:44:45AM +0200, Neil Armstrong wrote:
>> Add Synopsys Designware MAC Glue layer for the Oxford Semiconductor OX820.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  .../devicetree/bindings/net/oxnas-dwmac.txt        |  44 +++++
> 
> It's preferred that bindings are a separate patch.

OK

> 
>>  drivers/net/ethernet/stmicro/stmmac/Kconfig        |  11 ++
>>  drivers/net/ethernet/stmicro/stmmac/Makefile       |   1 +
>>  drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c  | 219 +++++++++++++++++++++
>>  4 files changed, 275 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/oxnas-dwmac.txt
>>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c
>>
>> Changes since RFC at https://patchwork.kernel.org/patch/9387257 :
>>  - Drop init/exit callbacks
>>  - Implement proper remove and PM callback
>>  - Call init from probe
>>  - Disable/Unprepare clock if stmmac probe fails
>>
>> diff --git a/Documentation/devicetree/bindings/net/oxnas-dwmac.txt b/Documentation/devicetree/bindings/net/oxnas-dwmac.txt
>> new file mode 100644
>> index 0000000..5d2696c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/oxnas-dwmac.txt
>> @@ -0,0 +1,44 @@
>> +* Oxford Semiconductor OXNAS DWMAC Ethernet controller
>> +
>> +The device inherits all the properties of the dwmac/stmmac devices
>> +described in the file stmmac.txt in the current directory with the
>> +following changes.
>> +
>> +Required properties on all platforms:
>> +
>> +- compatible:	Depending on the platform this should be one of:
>> +			- "oxsemi,ox820-dwmac"
>> +		Additionally "snps,dwmac" and any applicable more
>> +		detailed version number described in net/stmmac.txt
>> +		should be used.
> 
> You should be explicit what version applies to ox820. "snps,dwmac" 
> should probably be deprecated IMO. There are so many variations of DW 
> h/w.

Well, to be honest I have absolutely no idea ! But I will try to find out...

> 
>> +
>> +- reg:	The first register range should be the one of the DWMAC
>> +	controller.
> 
> This is worded like there's a 2nd range?

OK, will rephrase.

> 
>> +
>> +- clocks: Should contain phandles to the following clocks
>> +- clock-names:	Should contain the following:
>> +		- "stmmaceth" - see stmmac.txt
>> +		- "gmac" - peripheral gate clock
>> +
>> +- oxsemi,sys-ctrl: a phandle to the system controller syscon node
>> +
>> +Example :
>> +
>> +etha: ethernet@40400000 {
>> +	compatible = "oxsemi,ox820-dwmac", "snps,dwmac";
>> +	reg = <0x40400000 0x2000>;
>> +	interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
>> +		     <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
>> +	interrupt-names = "macirq", "eth_wake_irq";
>> +	mac-address = [000000000000]; /* Filled in by U-Boot */
>> +	phy-mode = "rgmii";
>> +
>> +	clocks = <&stdclk CLK_820_ETHA>, <&gmacclk>;
>> +	clock-names = "gmac", "stmmaceth";
>> +	resets = <&reset RESET_MAC>;
>> +
>> +	/* Regmap for sys registers */
>> +	oxsemi,sys-ctrl = <&sys>;
>> +
>> +	status = "disabled";
>> +};