[U-Boot] [PATCH v2 1/2] spi: nxp_fspi: new driver for the FlexSPI controller

[Priyanka Jain]

>-----Original Message-----
>From: Michael Walle <michael at walle.cc>
>Sent: Monday, December 16, 2019 6:50 PM
>To: Priyanka Jain <priyanka.jain at nxp.com>; Jagan Teki
><jagan at amarulasolutions.com>; Vignesh R <vigneshr at ti.com>
>Cc: u-boot at lists.denx.de
>Subject: Re: [U-Boot] [PATCH v2 1/2] spi: nxp_fspi: new driver for the FlexSPI
>controller
>
>Hi Priyanka, Hi Jagan, Hi Vingesh,
>
>Am 2019-11-29 08:49, schrieb Michael Walle:
>> Am 2019-11-29 06:03, schrieb Priyanka Jain:
>>>> -----Original Message-----
>>>> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Michael
>>>> Walle
>>>> Sent: Saturday, November 2, 2019 11:56 PM
>>>> To: u-boot at lists.denx.de
>>>> Subject: [U-Boot] [PATCH v2 1/2] spi: nxp_fspi: new driver for the
>>>> FlexSPI controller
>>>>
>>>> This is a port of the kernel's spi-nxp-fspi driver. It uses the new
>>>> spi-mem interface and does not expose the more generic spi-xfer
>>>> interface.
>>>> The source
>>>> was taken from the v5.3-rc3 tag.
>>>>
>>>> The port was straightforward:
>>>> - remove the interrupt handling and the completion by busy polling
>>>> the
>>>>   controller
>>>> - remove locks
>>>> - move the setup of the memory windows into claim_bus()
>>>> - move the setup of the speed into set_speed()
>>>> - port the device tree bindings from the original fspi_probe() to
>>>>   ofdata_to_platdata()
>>>>
>>>> There were only some style change fixes, no change in any logic. For
>>>> example, there are busy loops where the return code is not handled
>>>> correctly, eg. only prints a warning with WARN_ON(). This port
>>>> intentionally left most functions unchanged to ease future bugfixes.
>>>>
>>>> This was tested on a custom LS1028A board. Because the LS1028A
>>>> doesn't have proper clock framework support, changing the clock
>>>> speed was not tested. This also means that it is not possible to
>>>> change the SPI speed on LS1028A for now (neither is it possible in
>>>> the linux driver).
>>>>
>>>> Signed-off-by: Michael Walle <michael at walle.cc>
>>>> Reviewed-by: Jagan Teki <jagan at amarulasolutions.com>
>>>> ---
>>>> changes since v1:
>>>> - fixed typo, thanks Jagan
>>>>
>>>> drivers/spi/Kconfig    |   7 +
>>>> drivers/spi/Makefile   |   1 +
>>>> drivers/spi/nxp_fspi.c | 997
>>>> +++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 1005 insertions(+)
>>>> create mode 100644 drivers/spi/nxp_fspi.c
>>>>
>>>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
>>>> 7be867d5b6..ad20309df8 100644
>>>> --- a/drivers/spi/Kconfig
>>>> +++ b/drivers/spi/Kconfig
>>>> @@ -192,6 +192,13 @@ config MVEBU_A3700_SPI
>>>> 	  used to access the SPI NOR flash on platforms embedding this
>>>> 	  Marvell IP core.
>>>>
>>>> +config NXP_FSPI
>>>> +	bool "NXP FlexSPI driver"
>>>> +	depends on SPI_MEM
>>>> +	help
>>>> +	  Enable the NXP FlexSPI (FSPI) driver. This driver can be used to
>>>> +	  access the SPI NOR flash on platforms embedding this NXP IP core.
>>>> +
>>>> config PIC32_SPI
>>>> 	bool "Microchip PIC32 SPI driver"
>>>> 	depends on MACH_PIC32
>>>> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index
>>>> ae4f2958f8..52462e19a3 100644
>>>> --- a/drivers/spi/Makefile
>>>> +++ b/drivers/spi/Makefile
>>>> @@ -43,6 +43,7 @@ obj-$(CONFIG_MSCC_BB_SPI) += mscc_bb_spi.o
>>>> obj-$(CONFIG_MVEBU_A3700_SPI) += mvebu_a3700_spi.o
>>>> obj-$(CONFIG_MXC_SPI) += mxc_spi.o
>>>> obj-$(CONFIG_MXS_SPI) += mxs_spi.o
>>>> +obj-$(CONFIG_NXP_FSPI) += nxp_fspi.o
>>>> obj-$(CONFIG_ATCSPI200_SPI) += atcspi200_spi.o
>>>> obj-$(CONFIG_OMAP3_SPI) += omap3_spi.o
>>>> obj-$(CONFIG_PIC32_SPI) += pic32_spi.o diff --git
>>>> a/drivers/spi/nxp_fspi.c b/drivers/spi/nxp_fspi.c new file mode
>>>> 100644 index 0000000000..b808418eb6
>>>> --- /dev/null
>>>> +++ b/drivers/spi/nxp_fspi.c
>>>> @@ -0,0 +1,997 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * NXP FlexSPI(FSPI) controller driver.
>>>> + *
>>>> + * Copyright (c) 2019 Michael Walle <michael at walle.cc>
>>> The file is ported from Linux.
>>> Any particular reason of adding copyright?
>>
>> so what? the "original" copyright is mentioned. It is certainly not
>> the case that there wasn't any changes. I get the feeling that you
>> just want to have "your" NXP copyright here. If any, there should be
>> the copyright of the original spi-nxp-qspi.c driver, which is not NXP.
>> But instead NXP - lets say borrowed - much of its code for "their"
>> spi-nxp-fspi.c and _dropped_ all former copyrights.
>>
>>
>>> Can you change this to Author/Ported-by?
>> Nope.
>
I am not asking you to drop copyright.
It's a query to understand what value you are adding to the existing Linux driver. 
The process that I follow for  all copyright additions in case things are not very explicit.
>ping. there was already tested-by's and reviewd-by's.
>
>>
>> -michael
>>
>>>> + *
>>>> + * This driver was originally ported from the linux kernel
>>>> +v5.4-rc3, which had
>>>> + * the following notes:
>>>> + *
>>> <snip>
>
>btw the original copyright is here after the snip. FWIW i can move that one also
>to the top. but I certainly won't drop mine.
Please pull in original copyright to the top as well.
>
>-michael
-Priyanka