Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: RnaVAjoz7qlXYMO1U7Qf00Q1j9MjlGTJ5imBEoeQvtjHIpfDMXUeF8auplwCmIZVc3Kf61ZPrW
 7194go2GC/2b3tc8QV/pd+xedtcmeIiyBHKZcmiZ9lZ34WrUNApXEb4J2dEB1UWSCeiKMLjTS9
 sFLtmAFWo9CiK+BITa8mB7Ka7lCPxPG50pqHKEUar6yR5l6L2VF2UWL1gGPUjhoDslBxMWqIho
 EWs9ppZc7Yi+8JLhoT1SiMcuSSWFpdraxWoyDXOMjqCUQzaz/Sz+Es4OOBFoTrLAgnyEmQKbFl
 o9s=
Date:   Tue, 19 May 2020 14:05:19 +0200
From:   Lars Povlsen <lars.povlsen@microchip.com>
To:     Mark Brown <broonie@kernel.org>
CC:     SoC Team <soc@kernel.org>,
        Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
        <linux-spi@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Lars Povlsen <lars.povlsen@microchip.com>
Subject: Re: [PATCH 05/10] spi: spi-dw-mmio: Spin off MSCC platforms into
 spi-dw-mchp
Message-ID: <20200519120519.GE24801@soft-dev15.microsemi.net>
References: <20200513140031.25633-1-lars.povlsen@microchip.com>
 <20200513140031.25633-6-lars.povlsen@microchip.com>
 <20200513151811.GL4803@sirena.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20200513151811.GL4803@sirena.org.uk>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 13/05/20 16:18, Mark Brown wrote:
> Date: Wed, 13 May 2020 16:18:11 +0100
> From: Mark Brown <broonie@kernel.org>
> To: Lars Povlsen <lars.povlsen@microchip.com>
> Cc: SoC Team <soc@kernel.org>, Microchip Linux Driver Support
>  <UNGLinuxDriver@microchip.com>, linux-spi@vger.kernel.org,
>  devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
>  linux-arm-kernel@lists.infradead.org, Alexandre Belloni
>  <alexandre.belloni@bootlin.com>
> Subject: Re: [PATCH 05/10] spi: spi-dw-mmio: Spin off MSCC platforms into
>  spi-dw-mchp
> User-Agent: Mutt/1.10.1 (2018-07-13)
> 
> On Wed, May 13, 2020 at 04:00:26PM +0200, Lars Povlsen wrote:
> 
> > +config SPI_DW_MCHP
> > +	tristate "Memory-mapped io interface driver using DW SPI core of MSCC SoCs"
> > +	default y if ARCH_SPARX5
> > +	default y if SOC_VCOREIII
> 
> Why the default ys?

The SoC will typically boot from SPI... But its not a requirement per
se. I will remove it.

> 
> > +++ b/drivers/spi/Makefile
> > @@ -37,6 +37,7 @@ obj-$(CONFIG_SPI_DAVINCI)		+= spi-davinci.o
> >  obj-$(CONFIG_SPI_DLN2)			+= spi-dln2.o
> >  obj-$(CONFIG_SPI_DESIGNWARE)		+= spi-dw.o
> >  obj-$(CONFIG_SPI_DW_MMIO)		+= spi-dw-mmio.o
> > +obj-$(CONFIG_SPI_DW_MCHP)		+= spi-dw-mchp.o
> >  obj-$(CONFIG_SPI_DW_PCI)		+= spi-dw-midpci.o
> >  spi-dw-midpci-objs			:= spi-dw-pci.o spi-dw-mid.o
> >  obj-$(CONFIG_SPI_EFM32)			+= spi-efm32.o
> 
> Please keep the file alphabetically sorted.
> 

Noted.

> > +++ b/drivers/spi/spi-dw-mchp.c
> > @@ -0,0 +1,232 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Memory-mapped interface driver for MSCC SoCs
> > + *
> 
> Please make the entire comment a C++ one so things look more
> intentional.

Sure, I can do that. The presented form matches that of the other
spi-dw-* drivers, but I can see other using // blocks. Ack.

> 
> > +#define MAX_CS		4
> 
> This should be namespaced.

Ack.

> 
> > +	rx_sample_dly = 0;
> > +	device_property_read_u32(&pdev->dev, "spi-rx-delay-us", &rx_sample_dly);
> > +	dws->rx_sample_dly = DIV_ROUND_UP(rx_sample_dly,
> > +					  (dws->max_freq / 1000000));
> 
> If this is a standard feature of the DesignWare IP why parse it here and
> not in the generic code?

This is a standard feature of the DesignWare IP, so good suggestion. I
will arrange with Serge.

Thank you for your comments!

---Lars