mbox series

[v1,0/2] add support for Meson A1 SPI Flash Controller

Message ID 20230322150458.783901-1-mmkurbanov@sberdevices.ru
Headers show
Series add support for Meson A1 SPI Flash Controller | expand

Message

Martin Kurbanov March 22, 2023, 3:04 p.m. UTC
This patchset introduces DT bindings and driver for the Amlogic Meson A1
SPI flash controller (A113L family).

The existing spi-meson-spifc driver is incompatible with the A1 SPIFC
at all.

The implementation has been tested on the Amlogic A113L SoC based device
connected with ESMT F50L1G41LB spinand flash.

This patchset has dependencies on the A1 clock series which is still
under review [1].

Links:
  [1] https://lore.kernel.org/all/20230321193014.26349-1-ddrokosov@sberdevices.ru/

Martin Kurbanov (2):
  dt-bindings: spi: add binding for meson-spifc-a1
  spi: add support for Meson A1 SPI Flash Controller

 .../bindings/spi/amlogic,meson-a1-spifc.yaml  |  42 ++
 drivers/spi/Kconfig                           |   7 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-meson-spifc-a1.c              | 444 ++++++++++++++++++
 4 files changed, 494 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/amlogic,meson-a1-spifc.yaml
 create mode 100644 drivers/spi/spi-meson-spifc-a1.c

--
2.40.0

Comments

Neil Armstrong March 22, 2023, 4 p.m. UTC | #1
Hi,

On 22/03/2023 16:04, Martin Kurbanov wrote:
> This is a driver for the Amlogic Meson SPI flash controller support
> on A113L SoC.
> 
> Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru>
> ---
>   drivers/spi/Kconfig              |   7 +
>   drivers/spi/Makefile             |   1 +
>   drivers/spi/spi-meson-spifc-a1.c | 444 +++++++++++++++++++++++++++++++
>   3 files changed, 452 insertions(+)
>   create mode 100644 drivers/spi/spi-meson-spifc-a1.c
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 3b1c0878bb85..a12452bd1e0c 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -582,6 +582,13 @@ config SPI_MESON_SPIFC
>   	  This enables master mode support for the SPIFC (SPI flash
>   	  controller) available in Amlogic Meson SoCs.
>   
> +config SPI_MESON_SPIFC_A1
> +	tristate "Amlogic Meson A113L SPIFC controller"

The title should be "Amlogic Meson A1 SPIFC controller" for coherency.

> +	depends on ARCH_MESON || COMPILE_TEST
> +	help
> +	  This enables master mode support for the SPIFC (SPI flash
> +	  controller) available in Amlogic Meson A113L (A1 family) SoC.

You should write the reverse: available in Amlogic Meson A1 Family (A113L SoC).

> +
>   config SPI_MICROCHIP_CORE
>   	tristate "Microchip FPGA SPI controllers"
>   	depends on SPI_MASTER
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index be9ba40ef8d0..702053970967 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -72,6 +72,7 @@ obj-$(CONFIG_SPI_LM70_LLP)		+= spi-lm70llp.o
>   obj-$(CONFIG_SPI_LP8841_RTC)		+= spi-lp8841-rtc.o
>   obj-$(CONFIG_SPI_MESON_SPICC)		+= spi-meson-spicc.o
>   obj-$(CONFIG_SPI_MESON_SPIFC)		+= spi-meson-spifc.o
> +obj-$(CONFIG_SPI_MESON_SPIFC_A1)	+= spi-meson-spifc-a1.o
>   obj-$(CONFIG_SPI_MICROCHIP_CORE)	+= spi-microchip-core.o
>   obj-$(CONFIG_SPI_MICROCHIP_CORE_QSPI)	+= spi-microchip-core-qspi.o
>   obj-$(CONFIG_SPI_MPC512x_PSC)		+= spi-mpc512x-psc.o
> diff --git a/drivers/spi/spi-meson-spifc-a1.c b/drivers/spi/spi-meson-spifc-a1.c
> new file mode 100644
> index 000000000000..213c8b692675
> --- /dev/null
> +++ b/drivers/spi/spi-meson-spifc-a1.c
> @@ -0,0 +1,444 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Amlogic Meson A113L SPI flash controller (SPIFC)

Same here

> + *
> + * Copyright (c) 2023, SberDevices. All Rights Reserved.
> + *
> + * Author: Martin Kurbanov <mmkurbanov@sberdevices.ru>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>
> +#include <linux/types.h>
> +
> +#define A1_SPIFC_AHB_CTRL_REG		0x0
> +#define A1_SPIFC_AHB_BUS_EN		BIT(31)

I find the "A1_SPIFC" hard to read, I think you should reverse
it in all the file into :
#define SPIFC_A1_...
and
static XXX meson_spifc_a1_request to be coherent with the legacy spifc
driver and spicc driver.


<snip>

> +
> +MODULE_AUTHOR("Martin Kurbanov <mmkurbanov@sberdevices.ru>");
> +MODULE_DESCRIPTION("Amlogic Meson A113L SPIFC driver");

Same here "Meson A1 SPIFC driver"

> +MODULE_LICENSE("GPL");

Thanks,
Neil