Add SPI control driver for Sunplus SP7021 SoC
Subsystems (3)
Li-hao Kuo
maintainerv5 cover Add SPI control driver for Sunplus SP7021 SoC
This is a patch series for SPI driver for Sunplus SP7021 SoC.
Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD card and etc.) into a single chip. It is designed for industrial control.
Refer to:
https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
https://tibbo.com/store/plus1.html
Li-hao Kuo (2):
SPI: Add SPI driver for Sunplus SP7021
devicetree: bindings SPI Add bindings doc for Sunplus SP7021
.../bindings/spi/spi-sunplus-sp7021.yaml | 81 +++ MAINTAINERS | 7 + drivers/spi/Kconfig | 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-sunplus_sp7021.c | 603 +++++++++++++++++++++ 5 files changed, 703 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml create mode 100644 drivers/spi/spi-sunplus_sp7021.c
Signature · 1 line
2.7.4
Li-hao Kuo
maintainerPATCH v5 1/2 SPI: Add SPI driver for Sunplus SP7021
Add SPI driver for Sunplus SP7021.
---
Changes in v5:
- Addressed comments from Mr. Mark Brown
- Addressed comments from Mr. Andy Shevchenko
MAINTAINERS | 6 + drivers/spi/Kconfig | 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-sunplus_sp7021.c | 603 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 621 insertions(+) create mode 100644 drivers/spi/spi-sunplus_sp7021.c
MAINTAINERS +6 -0
@@ -18243,6 +18243,12 @@ L: [email protected] S: Maintained F: drivers/net/ethernet/dlink/sundance.c +SUNPLUS SPI CONTROLLER INTERFACE DRIVER +M: Li-hao Kuo <[email protected]> +L: [email protected] +S: Maintained +F: drivers/spi/spi-sunplus-sp7021.c + SUPERH M: Yoshinori Sato <[email protected]> M: Rich Felker <[email protected]>
drivers/spi/Kconfig +11 -0
@@ -866,6 +866,17 @@ config SPI_SUN6I help This enables using the SPI controller on the Allwinner A31 SoCs. +config SPI_SUNPLUS_SP7021 + tristate "Sunplus SP7021 SPI controller" + depends on SOC_SP7021 || COMPILE_TEST + help + This enables Sunplus SP7021 SPI controller driver on the SP7021 SoCs. + This driver can also be built as a module. If so, the module will be + called as spi-sunplus-sp7021. + + If you have a Sunplus SP7021 platform say Y here. + If unsure, say N. + config SPI_SYNQUACER tristate "Socionext's SynQuacer HighSpeed SPI controller" depends on ARCH_SYNQUACER || COMPILE_TEST
drivers/spi/Makefile +1 -0
@@ -119,6 +119,7 @@ obj-$(CONFIG_SPI_STM32_QSPI) += spi-stm32-qspi.o obj-$(CONFIG_SPI_ST_SSC4) += spi-st-ssc4.o obj-$(CONFIG_SPI_SUN4I) += spi-sun4i.o obj-$(CONFIG_SPI_SUN6I) += spi-sun6i.o +obj-$(CONFIG_SPI_SUNPLUS_SP7021) += spi-sunplus-sp7021.o obj-$(CONFIG_SPI_SYNQUACER) += spi-synquacer.o obj-$(CONFIG_SPI_TEGRA210_QUAD) += spi-tegra210-quad.o obj-$(CONFIG_SPI_TEGRA114) += spi-tegra114.o
drivers/spi/spi-sunplus_sp7021.c new file +603 -0
603 lines not on this page · raw message
Signature · 1 line
2.7.4
Re: [PATCH v5 1/2] SPI: Add SPI driver for Sunplus SP7021
Quoted 2 lines · Li-hao Kuo
Add SPI driver for Sunplus SP7021.
In the subject line use small letters in the prefix. Check with git log -- drivers/spi how people do.
Common comment: Consider to use spi_controller_*() APIs over spi_master_*() ones. Also your SLA/MAS (and sla/mas) are a bit
confusing: spell them in full and master --> controller or ctrl, slave --> peripheral or alike.
Quoted 5 lines
Signed-off-by Li-hao Kuo <[email protected]>---
Changes in v5:
- Addressed comments from Mr. Mark Brown
- Addressed comments from Mr. Andy Shevchenko
You need to elaborate what exactly you addressed.
...
drivers/spi/spi-sunplus_sp7021.c line 237
Quoted 2 lines
drivers/spi/spi-sunplus_sp7021.c new file +2
-0+writel (readl (pspim->m_base +SP7021_INT_BUSY_REG ) + |SP7021_CLR_MAS_INT , pspim->m_base +SP7021_INT_BUSY_REG );
It's better to read with temporary variable being used:
value = readl(pspim->m_base + SP7021_INT_BUSY_REG);
value |= SP7021_CLR_MAS_INT;
writel(value, pspim->m_base + SP7021_INT_BUSY_REG);...
drivers/spi/spi-sunplus_sp7021.c line 267
Quoted 2 lines
drivers/spi/spi-sunplus_sp7021.c new file +2
-0+writel (readl (pspim->m_base +SP7021_SPI_STATUS_REG ) |SP7021_FD_SW_RST , + pspim->m_base +SP7021_SPI_STATUS_REG );
Ditto. And for all other similar cases.
...
Quoted 1 line
+
pspim->xfer_conf|= ((clk_sel&0xffff) << 16);
Is xfer_conf bigger than 32-bit? If not, why do you need the ' & 0xffff' part?
...
Quoted 1 line
+ ret = 0;
Is it necessary to do this under the lock?
Quoted 4 lines
drivers/spi/spi-sunplus_sp7021.c new file +4
-0+if (pspim->xfer_conf &SP7021_CPOL_FD ) +writel (pspim->xfer_conf, pspim->m_base +SP7021_SPI_CONFIG_REG ); + +mutex_unlock (&pspim->buf_lock);
...
Quoted 1 line
+ if (
spi_controller_is_slave(ctlr)) {
Factor out this body to a function, it will increase readability.
Quoted 4 lines
drivers/spi/spi-sunplus_sp7021.c new file +4
-0+ } + +spi_finalize_current_transfer (ctlr); +return ret;
...
Quoted 1 line
+ mode =
SP7021_MASTER_MODE;
This...
drivers/spi/spi-sunplus_sp7021.c line 439
Quoted 4 lines
drivers/spi/spi-sunplus_sp7021.c new file +3
-0+ pdev->id =of_alias_get_id (pdev->dev.of_node,"sp_spi" ); +if (of_property_read_bool (pdev->dev.of_node,"spi-slave" )) + mode =SP7021_SLAVE_MODE ;
...belongs to this condition, so do not interleave them.
On top of that you may use device property API:
if (device_property_read_bool(&pdev->dev, "spi-slave"))
mode = SP7021_SLAVE_MODE;
else
mode = SP7021_MASTER_MODE;...
drivers/spi/spi-sunplus_sp7021.c line 530
Quoted 8 lines
drivers/spi/spi-sunplus_sp7021.c new file +8
-0+pm_runtime_enable (dev); + ret =spi_register_controller (ctlr); +if (ret) { +pm_runtime_disable (dev); +return dev_err_probe (dev, ret,"spi_register_master fail\n" ); + } + +return ret;
return 0;
...
Quoted 1 line
+MODULE_LICENSE("GPL v2");
"GPL", the one you used is legacy.
Signature · 2 lines
With Best Regards, Andy Shevchenko
RE: [PATCH v5 1/2] SPI: Add SPI driver for Sunplus SP7021
Quoted 9 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ writel(readl(pspim->m_base + SP7021_INT_BUSY_REG) + | SP7021_CLR_MAS_INT, pspim->m_base + + SP7021_INT_BUSY_REG);It's better to read with temporary variable being used:
value = readl(pspim->m_base + SP7021_INT_BUSY_REG); value |= SP7021_CLR_MAS_INT; writel(value, pspim->m_base + SP7021_INT_BUSY_REG);
I will add it in next next submission
Quoted 5 lines
Quoted 1 line
+
pspim->xfer_conf|= ((clk_sel&0xffff) << 16);Is
xfer_confbigger than 32-bit? If not, why do you need the ' &0xffff' part?...
I will add it in next next submission
Quoted 10 lines
Quoted 1 line
+ mode =
SP7021_SLAVE_MODE;...belongs to this condition, so do not interleave them.
On top of that you may use device property API:
if (device_property_read_bool(&pdev->dev, "spi-slave")) mode = SP7021_SLAVE_MODE; else mode = SP7021_MASTER_MODE;
I will add it in next next submission
Quoted 5 lines
drivers/spi/spi-sunplus_sp7021.c line 530
Quoted 4 lines
drivers/spi/spi-sunplus_sp7021.c new file +4
-0+return dev_err_probe (dev, ret,"spi_register_master fail\n" ); + } + +return ret;
Quoted 8 lines
return 0;
...
Quoted 1 line
+MODULE_LICENSE("GPL v2");
"GPL", the one you used is legacy.
I will add it in next next submission
kernel test robot
build botRe: [PATCH v5 1/2] SPI: Add SPI driver for Sunplus SP7021
In reply to Li-hao KuoI love your patch! Yet something to improve:
Hi Li-hao,
I love your patch! Yet something to improve:
[auto build test ERROR on broonie-spi/for-next]
[also build test ERROR on pza/reset/next robh/for-next linus/master v5.16] [cannot apply to next-20220110]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Li-hao-Kuo/Add-SPI-control-driver-for-Sunplus-SP7021-SoC/20220110-144745 base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220110/202201101911.hSqY5DkA-lkp(a)intel.com/config) compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/53da1eda2d6e7a04e09dcb37a029ec3b384b7afb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Li-hao-Kuo/Add-SPI-control-driver-for-Sunplus-SP7021-SoC/20220110-144745 git checkout 53da1eda2d6e7a04e09dcb37a029ec3b384b7afb
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
>> make[3]: *** No rule to make target 'drivers/spi/spi-sunplus-sp7021.o', needed by 'drivers/spi/built-in.a'.
make[3]: Target '__build' not remade because of errors.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Li-hao Kuo
maintainerPATCH v5 2/2 devicetree: bindings SPI Add bindings doc for Sunplus SP7021
In reply to Li-hao KuoAdd devicetree bindings SPI Add bindings doc for Sunplus SP7021
Reviewed by Mr. Rob Herring <[email protected]>
---
Changes in v5:
- no change.
- Reviewed by Mr. Rob Herring <[email protected]>
.../bindings/spi/spi-sunplus-sp7021.yaml | 81 ++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 82 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml
Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml new file +81 -0
@@ -0,0 +1,81 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright (C) Sunplus Co., Ltd. 2021 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/spi-sunplus-sp7021.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sunplus sp7021 SPI controller + +allOf: + - $ref: "spi-controller.yaml" + +maintainers: + - Li-hao Kuo <[email protected]> + +properties: + compatible: + enum: + - sunplus,sp7021-spi + + reg: + items: + - the SPI master registers + - the SPI slave registers + + reg-names: + items: + - const: master + - const: slave + + interrupt-names: + items: + - const: dma_w + - const: mas_risc + - const: slave_risc + + interrupts: + minItems: 3 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + +required: + - compatible + - reg + - reg-names + - interrupts + - interrupt-names + - clocks + - clocks-names + - resets + - pinctrl-names + - pinctrl-0 + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/sp-sp7021.h> + #include <dt-bindings/reset/sp-sp7021.h> + #include <dt-bindings/interrupt-controller/irq.h> + spi@9C002D80 { + compatible = "sunplus,sp7021-spi"; + reg = <0x9C002D80 0x80>, <0x9C002E00 0x80>; + reg-names = "master", "slave"; + interrupt-parent = <&intc>; + interrupt-names = "dma_w", + "mas_risc", + "slave_risc"; + interrupts = <144 IRQ_TYPE_LEVEL_HIGH>, + <146 IRQ_TYPE_LEVEL_HIGH>, + <145 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clkc SPI_COMBO_0>; + resets = <&rstc RST_SPI_COMBO_0>; + pinctrl-names = "default"; + pinctrl-0 = <&pins_spi0>; + }; +...
MAINTAINERS +1 -0
@@ -18247,6 +18247,7 @@ SUNPLUS SPI CONTROLLER INTERFACE DRIVER M: Li-hao Kuo <[email protected]> L: [email protected] S: Maintained +F: Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml F: drivers/spi/spi-sunplus-sp7021.c SUPERH
Signature · 1 line
2.7.4
Re: [PATCH v5 2/2] devicetree: bindings SPI Add bindings doc for Sunplus SP7021
Quoted 3 lines · Li-hao Kuo
Add devicetree bindings SPI Add bindings doc for Sunplus SP7021
Reviewed by Mr. Rob Herring <[email protected]>
Tag is wrong in multiple ways. It must be exactly what I gave:
Quoted 114 lines
Signed-off-by Li-hao Kuo <[email protected]>---
Changes in v5:
- no change.
- Reviewed by Mr. Rob Herring <[email protected]>.../bindings/spi/spi-sunplus-sp7021.yaml | 81 ++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 82 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yamlDocumentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml new file +81
-0@@ -0,0 +1,81 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright (C) Sunplus Co., Ltd. 2021 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/spi-sunplus-sp7021.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sunplus sp7021 SPI controller + +allOf: + - $ref: "spi-controller.yaml" + +maintainers: + - Li-hao Kuo <[email protected]> + +properties: + compatible: + enum: + - sunplus,sp7021-spi + + reg: + items: + - the SPI master registers + - the SPI slave registers + + reg-names: + items: + - const: master + - const: slave + + interrupt-names: + items: + - const: dma_w + - const: mas_risc + - const: slave_risc + + interrupts: + minItems: 3 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + +required: + - compatible + - reg + - reg-names + - interrupts + - interrupt-names + - clocks + - clocks-names + - resets + - pinctrl-names + - pinctrl-0 + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/sp-sp7021.h> + #include <dt-bindings/reset/sp-sp7021.h> + #include <dt-bindings/interrupt-controller/irq.h> + spi@9C002D80 { + compatible = "sunplus,sp7021-spi"; + reg = <0x9C002D80 0x80>, <0x9C002E00 0x80>; + reg-names = "master", "slave"; + interrupt-parent = <&intc>; + interrupt-names = "dma_w", + "mas_risc", + "slave_risc"; + interrupts = <144 IRQ_TYPE_LEVEL_HIGH>, + <146 IRQ_TYPE_LEVEL_HIGH>, + <145 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clkc SPI_COMBO_0>; + resets = <&rstc RST_SPI_COMBO_0>; + pinctrl-names = "default"; + pinctrl-0 = <&pins_spi0>; + }; +...MAINTAINERS +1
-0@@ -18247,6 +18247,7 @@ SUNPLUS SPI CONTROLLER INTERFACE DRIVER M: Li-hao Kuo <[email protected]> L: [email protected] S: Maintained +F: Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml F: drivers/spi/spi-sunplus-sp7021.c SUPERHSignature · 1 line
2.7.4