* [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi @ 2023-05-22 7:10 Yinbo Zhu 2023-05-22 7:10 ` [PATCH v11 1/2] dt-bindings: spi: add " Yinbo Zhu ` (3 more replies) 0 siblings, 4 replies; 27+ messages in thread From: Yinbo Zhu @ 2023-05-22 7:10 UTC (permalink / raw) To: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Yinbo Zhu Loongson platform support spi hardware controller and this series patch was to add spi driver and binding support. Change in v2: 1. This [PATCH v2 1/2] dt-bindings patch need depend on clk patch: https:// lore.kernel.org/all/20230307115022.12846-1-zhuyinbo@loongson.cn/ 2. Remove the clock-names in spi yaml file. 3. Add "loongson,ls7a-spi" compatible in spi yaml file. 4. Add an || COMPILE_TEST and drop && PCI then add some CONFIG_PCI macro to limit some pci code. 5. Make the spi driver top code comment block that use C++ style. 6. Drop spi->max_speed_hz. 7. Add a spin_lock for loongson_spi_setup. 8. Add a timeout and cpu_relax() in loongson_spi_write_read_8bit. 9. Add spi_transfer_one and drop transfer and rework entire spi driver that include some necessary changes. 10. Use module_init replace subsys_initcall. 11. About PM interface that I don't find any issue so I don't add any changes. Change in v3: 1. This [PATCH v3 1/2] dt-bindings patch need depend on clk patch: https:// lore.kernel.org/all/20230323025229.2971-1-zhuyinbo@loongson.cn/ 2. Drop the unused blank line in loongson,ls-spi.yaml file. 3. Replace clock minItems with clock maxItems in yaml file. 4. Separate spi driver into platform module, pci module and core module. 5. Replace DIV_ROUND_UP with DIV_ROUND_UP_ULL to fix compile error "undefined reference to `__aeabi_uldivmod'" and "__udivdi3 undefined" that reported by test robot. 6. Remove the spin lock. 7. Clear the loongson_spi->hz and loongson_spi->mode in setup to fixup the issue that multiple spi device transfer that maybe cause spi was be misconfigured. Change in v4: 1. This [PATCH v4 1/2] dt-bindings patch need depend on clk patch: https:// lore.kernel.org/all/20230323025229.2971-1-zhuyinbo@loongson.cn/ 2. Add "#include <linux/io.h>" in spi-loongson-core.c for fix the compile issue which devm_ioremap no declaration. 3. Add "EXPORT_SYMBOL_GPL(loongson_spi_dev_pm_ops)" in spi-loongson-core.c for fix the compile issue which loongson_spi_dev_pm_ops undefined. Change in v5: 1. Get rid of the clock patch's dependency and open-code the clock IDs. 2. Fixup checkpatch issue that by installed ply and gitpython package locally, but this series of patch's code doesn't have any change. Change in v6: 1. Remove the "#include <dt-bindings/clock/loongson,ls2k-clk.h>" in yaml file. Change in v7: 1. Remove the "loongson,ls7a-spi" and change yaml file name as "loongson,ls2k-spi.yaml". 2. Use module_pci_driver and module_platform_driver to replace module_init and module_exit. 3. Drop ".owner = THIS_MODULE" in spi platform driver. 4. Add devm_spi_alloc_master devm_spi_register_master to simplify code. 5. Add pci_disable_device() in loongson_spi_pci_unregister. Change in v8: 1. Add reviewed-by information for spi bindings patch. 2. Fixup the uncorrect spi yaml file path in MAINTAINERS file. 3. Add spi_master_suspend and spi_master_resume in spi pm function. Change in v9: 1. Make spi_master_suspend go first in pm suspend. Change in v10: 1. Fix the compile issue about of_node_get and of_get_property no declaration. 2. set config SPI_LOONGSON_CORE invisible. 3. Captial "spi" in commit log and Kconfig file. 4. Write header files in alphabetical order. 5. Use clamp_val, GENMASK() and BIT() in spi clock setting. 6. Optimize clock and mode setting code. 7. Use readb_poll_timeout in loongson_spi_write_read_8bit. 8. Remove some useless dmesg print. 9. Use device_set_node replace of_node_get. 10. Use dev_err_probe in code. 11. Use devm_clk_get_optional replace devm_clk_get. 12. Remove SPI_NO_CS for drop 2k500 non common type spi. 13. Use pcim_enable_device() and pcim_iomap_regions() in spi pci driver. 14. Passing the remapped address in loongson_spi_init_master. 15. Remove the useless goto flag "err_out". 16. Use pci vendor id in pci_ids.h. 17. Use devm_platform_ioremap_resource in spi platform driver. 18. Remove the useless item in pci_device_id. 19. Remove the inned comma in of_device_id. 20. Add some headfile in spi_loongson.h. 21. Remove the useless extern for loongson_spi_init_master in spi_loongson.h. Change in v11: 1. Use spi_get_chipselect() to replace all spi->chip_select in spi driver Yinbo Zhu (2): dt-bindings: spi: add loongson spi spi: loongson: add bus driver for the loongson spi controller .../bindings/spi/loongson,ls2k-spi.yaml | 41 +++ MAINTAINERS | 10 + drivers/spi/Kconfig | 26 ++ drivers/spi/Makefile | 3 + drivers/spi/spi-loongson-core.c | 279 ++++++++++++++++++ drivers/spi/spi-loongson-pci.c | 61 ++++ drivers/spi/spi-loongson-plat.c | 46 +++ drivers/spi/spi-loongson.h | 47 +++ 8 files changed, 513 insertions(+) -- 2.20.1 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v11 1/2] dt-bindings: spi: add loongson spi 2023-05-22 7:10 [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi Yinbo Zhu @ 2023-05-22 7:10 ` Yinbo Zhu 2023-05-24 8:56 ` Conor Dooley 2023-05-22 7:10 ` [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller Yinbo Zhu ` (2 subsequent siblings) 3 siblings, 1 reply; 27+ messages in thread From: Yinbo Zhu @ 2023-05-22 7:10 UTC (permalink / raw) To: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Yinbo Zhu, Krzysztof Kozlowski Add the Loongson platform spi binding with DT schema format using json-schema. Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- .../bindings/spi/loongson,ls2k-spi.yaml | 41 +++++++++++++++++++ MAINTAINERS | 6 +++ 2 files changed, 47 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml new file mode 100644 index 000000000000..d0be6e5378d7 --- /dev/null +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Loongson SPI controller + +maintainers: + - Yinbo Zhu <zhuyinbo@loongson.cn> + +allOf: + - $ref: /schemas/spi/spi-controller.yaml# + +properties: + compatible: + enum: + - loongson,ls2k-spi + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + +required: + - compatible + - reg + - clocks + +unevaluatedProperties: false + +examples: + - | + spi0: spi@1fff0220{ + compatible = "loongson,ls2k-spi"; + reg = <0x1fff0220 0x10>; + clocks = <&clk 17>; + #address-cells = <1>; + #size-cells = <0>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index fea4317faf75..e49c04c53c99 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12201,6 +12201,12 @@ F: Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml F: drivers/clk/clk-loongson2.c F: include/dt-bindings/clock/loongson,ls2k-clk.h +LOONGSON SPI DRIVER +M: Yinbo Zhu <zhuyinbo@loongson.cn> +L: linux-spi@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml + LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) M: Sathya Prakash <sathya.prakash@broadcom.com> M: Sreekanth Reddy <sreekanth.reddy@broadcom.com> -- 2.20.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: spi: add loongson spi 2023-05-22 7:10 ` [PATCH v11 1/2] dt-bindings: spi: add " Yinbo Zhu @ 2023-05-24 8:56 ` Conor Dooley 2023-05-24 9:44 ` zhuyinbo 0 siblings, 1 reply; 27+ messages in thread From: Conor Dooley @ 2023-05-24 8:56 UTC (permalink / raw) To: Yinbo Zhu Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Krzysztof Kozlowski [-- Attachment #1: Type: text/plain, Size: 1584 bytes --] On Mon, May 22, 2023 at 03:10:29PM +0800, Yinbo Zhu wrote: > Add the Loongson platform spi binding with DT schema format using > json-schema. > > Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > .../bindings/spi/loongson,ls2k-spi.yaml | 41 +++++++++++++++++++ > MAINTAINERS | 6 +++ > 2 files changed, 47 insertions(+) > create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml > > diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml > new file mode 100644 > index 000000000000..d0be6e5378d7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml > @@ -0,0 +1,41 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Loongson SPI controller > + > +maintainers: > + - Yinbo Zhu <zhuyinbo@loongson.cn> > + > +allOf: > + - $ref: /schemas/spi/spi-controller.yaml# > + > +properties: > + compatible: > + enum: > + - loongson,ls2k-spi I am sorry to jump in here at such a late stage with a (potentially) trivial question. "ls2k" is the SoC family rather than a specific model as far as I understand. The answer is probably yes, but do all SoCs in the family have an identical version of the IP? Cheers, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: spi: add loongson spi 2023-05-24 8:56 ` Conor Dooley @ 2023-05-24 9:44 ` zhuyinbo 2023-05-24 10:29 ` Conor Dooley 0 siblings, 1 reply; 27+ messages in thread From: zhuyinbo @ 2023-05-24 9:44 UTC (permalink / raw) To: Conor Dooley Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Krzysztof Kozlowski, zhuyinbo 在 2023/5/24 下午4:56, Conor Dooley 写道: > On Mon, May 22, 2023 at 03:10:29PM +0800, Yinbo Zhu wrote: >> Add the Loongson platform spi binding with DT schema format using >> json-schema. >> >> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- >> .../bindings/spi/loongson,ls2k-spi.yaml | 41 +++++++++++++++++++ >> MAINTAINERS | 6 +++ >> 2 files changed, 47 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >> >> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >> new file mode 100644 >> index 000000000000..d0be6e5378d7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >> @@ -0,0 +1,41 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Loongson SPI controller >> + >> +maintainers: >> + - Yinbo Zhu <zhuyinbo@loongson.cn> >> + >> +allOf: >> + - $ref: /schemas/spi/spi-controller.yaml# >> + >> +properties: >> + compatible: >> + enum: >> + - loongson,ls2k-spi > > I am sorry to jump in here at such a late stage with a (potentially) > trivial question. "ls2k" is the SoC family rather than a specific model > as far as I understand. > The answer is probably yes, but do all SoCs in the family have an > identical version of the IP? No, but the spi supported by this loongson spi driver are all the same identical version, and other type or verion spi will be supported as needed in the future. Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: spi: add loongson spi 2023-05-24 9:44 ` zhuyinbo @ 2023-05-24 10:29 ` Conor Dooley 2023-05-25 2:22 ` zhuyinbo 0 siblings, 1 reply; 27+ messages in thread From: Conor Dooley @ 2023-05-24 10:29 UTC (permalink / raw) To: zhuyinbo Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Krzysztof Kozlowski [-- Attachment #1: Type: text/plain, Size: 2288 bytes --] On Wed, May 24, 2023 at 05:44:38PM +0800, zhuyinbo wrote: > > > 在 2023/5/24 下午4:56, Conor Dooley 写道: > > On Mon, May 22, 2023 at 03:10:29PM +0800, Yinbo Zhu wrote: > > > Add the Loongson platform spi binding with DT schema format using > > > json-schema. > > > > > > Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > > --- > > > .../bindings/spi/loongson,ls2k-spi.yaml | 41 +++++++++++++++++++ > > > MAINTAINERS | 6 +++ > > > 2 files changed, 47 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml > > > new file mode 100644 > > > index 000000000000..d0be6e5378d7 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml > > > @@ -0,0 +1,41 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Loongson SPI controller > > > + > > > +maintainers: > > > + - Yinbo Zhu <zhuyinbo@loongson.cn> > > > + > > > +allOf: > > > + - $ref: /schemas/spi/spi-controller.yaml# > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - loongson,ls2k-spi > > > > I am sorry to jump in here at such a late stage with a (potentially) > > trivial question. "ls2k" is the SoC family rather than a specific model > > as far as I understand. > > The answer is probably yes, but do all SoCs in the family have an > > identical version of the IP? > > > No, but the spi supported by this loongson spi driver are all the same > identical version, and other type or verion spi will be supported as > needed in the future. Does having a catch-all compatible make sense then when not all SoCs in the ls2k family will actually be able to use this driver? Or am I misunderstanding and all ls2k SoCs do work with this driver and you were talking about other, future products? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: spi: add loongson spi 2023-05-24 10:29 ` Conor Dooley @ 2023-05-25 2:22 ` zhuyinbo [not found] ` <2196dd29-93ee-00f7-65b4-ede73aa8ba77@linaro.org> [not found] ` <69d355ff-90e1-09d2-d4ff-0d7dedc8addb@linaro.org> 0 siblings, 2 replies; 27+ messages in thread From: zhuyinbo @ 2023-05-25 2:22 UTC (permalink / raw) To: Conor Dooley Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Krzysztof Kozlowski, zhuyinbo 在 2023/5/24 下午6:29, Conor Dooley 写道: > On Wed, May 24, 2023 at 05:44:38PM +0800, zhuyinbo wrote: >> >> >> 在 2023/5/24 下午4:56, Conor Dooley 写道: >>> On Mon, May 22, 2023 at 03:10:29PM +0800, Yinbo Zhu wrote: >>>> Add the Loongson platform spi binding with DT schema format using >>>> json-schema. >>>> >>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> >>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>> --- >>>> .../bindings/spi/loongson,ls2k-spi.yaml | 41 +++++++++++++++++++ >>>> MAINTAINERS | 6 +++ >>>> 2 files changed, 47 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >>>> new file mode 100644 >>>> index 000000000000..d0be6e5378d7 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >>>> @@ -0,0 +1,41 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Loongson SPI controller >>>> + >>>> +maintainers: >>>> + - Yinbo Zhu <zhuyinbo@loongson.cn> >>>> + >>>> +allOf: >>>> + - $ref: /schemas/spi/spi-controller.yaml# >>>> + >>>> +properties: >>>> + compatible: >>>> + enum: >>>> + - loongson,ls2k-spi >>> >>> I am sorry to jump in here at such a late stage with a (potentially) >>> trivial question. "ls2k" is the SoC family rather than a specific model >>> as far as I understand. >>> The answer is probably yes, but do all SoCs in the family have an >>> identical version of the IP? >> >> >> No, but the spi supported by this loongson spi driver are all the same >> identical version, and other type or verion spi will be supported as >> needed in the future. > > Does having a catch-all compatible make sense then when not all SoCs in > the ls2k family will actually be able to use this driver? Yes, it is make sense as it can reduce the workload of the community. For the Loongson platform, the versions of spi peripherals are almost the same, except for a few or individual SoCs. And we have also discussed compatible internally, and we tend to define it this way. > Or am I misunderstanding and all ls2k SoCs do work with this driver and > you were talking about other, future products? Actually, in 2k500 has one special type spi was only one cs and their's register definition was different from common type spi thus this driver doesn't support but this driver can support another common type spi in 2k500. for this special type spi I will add support as needed in the future. Thanks, Yinbo. > ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <2196dd29-93ee-00f7-65b4-ede73aa8ba77@linaro.org>]
* Re: [PATCH v11 1/2] dt-bindings: spi: add loongson spi [not found] ` <2196dd29-93ee-00f7-65b4-ede73aa8ba77@linaro.org> @ 2023-06-01 9:51 ` zhuyinbo 2023-06-01 15:30 ` Krzysztof Kozlowski 0 siblings, 1 reply; 27+ messages in thread From: zhuyinbo @ 2023-06-01 9:51 UTC (permalink / raw) To: Krzysztof Kozlowski, Conor Dooley Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/6/1 上午3:46, Krzysztof Kozlowski 写道: > On 25/05/2023 04:22, zhuyinbo wrote: >> >> >> 在 2023/5/24 下午6:29, Conor Dooley 写道: >>> On Wed, May 24, 2023 at 05:44:38PM +0800, zhuyinbo wrote: >>>> >>>> >>>> 在 2023/5/24 下午4:56, Conor Dooley 写道: >>>>> On Mon, May 22, 2023 at 03:10:29PM +0800, Yinbo Zhu wrote: >>>>>> Add the Loongson platform spi binding with DT schema format using >>>>>> json-schema. >>>>>> >>>>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> >>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>>> --- >>>>>> .../bindings/spi/loongson,ls2k-spi.yaml | 41 +++++++++++++++++++ >>>>>> MAINTAINERS | 6 +++ >>>>>> 2 files changed, 47 insertions(+) >>>>>> create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >>>>>> new file mode 100644 >>>>>> index 000000000000..d0be6e5378d7 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >>>>>> @@ -0,0 +1,41 @@ >>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>>>> +%YAML 1.2 >>>>>> +--- >>>>>> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml# >>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>>> + >>>>>> +title: Loongson SPI controller >>>>>> + >>>>>> +maintainers: >>>>>> + - Yinbo Zhu <zhuyinbo@loongson.cn> >>>>>> + >>>>>> +allOf: >>>>>> + - $ref: /schemas/spi/spi-controller.yaml# >>>>>> + >>>>>> +properties: >>>>>> + compatible: >>>>>> + enum: >>>>>> + - loongson,ls2k-spi >>>>> >>>>> I am sorry to jump in here at such a late stage with a (potentially) >>>>> trivial question. "ls2k" is the SoC family rather than a specific model >>>>> as far as I understand. >>>>> The answer is probably yes, but do all SoCs in the family have an >>>>> identical version of the IP? >>>> >>>> >>>> No, but the spi supported by this loongson spi driver are all the same >>>> identical version, and other type or verion spi will be supported as >>>> needed in the future. >>> >>> Does having a catch-all compatible make sense then when not all SoCs in >>> the ls2k family will actually be able to use this driver? >> >> >> Yes, it is make sense as it can reduce the workload of the community. >> For the Loongson platform, the versions of spi peripherals are almost >> the same, except for a few or individual SoCs. And we have also >> discussed compatible internally, and we tend to define it this way. > > So you have chosen different path than what's clearly recommended by > community, existing experience and documentation: > > https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 > > Family names are not accepted as specific compatibles. Whenever they > were accepted, it lead to problems. All the time. Thank you for your documentation and advice and the Loongson platform have loongson-2h (ls2h), loongson-2k (ls2k), loongson-2p (ls2p) or other series SoC, which loongson-2 seems to be the family name you mentioned and the "loongson,ls2k-spi" should be a speific compatible name. > > https://lore.kernel.org/all/20220822181701.GA89665-robh@kernel.org/ > https://lore.kernel.org/all/78651e07-6b3e-4243-8e1f-fcd1dfb3ffe1@www.fastmail.com/ > https://lore.kernel.org/all/288f56ba9cfad46354203b7698babe91@walle.cc/ > https://lore.kernel.org/all/106e443a-e765-51fe-b556-e4e7e2aa771c@linaro.org/ > and many many more discussions. > > You should choose carefully, because we will keep NAK-ing adding > properties to circumvent missing compatibles. I have read the documention and patch link that you mentioned and it seems to advice that We don't have wildcard names in the compatible string and use wildcard names that will cause issue. and the compatible "loongson,ls2k-spi" that wasn't a wildcard names, and if the loongson-2k spi controller hardware upgraded or changed the I will use "loongson,ls2k-spi-version" as a compatible, such as, "loongson,ls2k-spi-v1.1", "loongson,ls2k-spi-v1.1a" or other. >> >>> Or am I misunderstanding and all ls2k SoCs do work with this driver and >>> you were talking about other, future products? >> >> Actually, in 2k500 has one special type spi was only one cs and their's >> register definition was different from common type spi thus this driver >> doesn't support but this driver can support another common type spi in >> 2k500. for this special type spi I will add support as needed in the >> future. > > Bindings are for hardware, not driver. What does your driver support or > does not, matters less. okay, I got it, and the loongson spi bindings was for loongson spi controller hardware. if the spi controller hardware not changed in different ls2k SoC and the spi compatible should be same thus loongson spi compatible seems to be adhere to the bindings aggrement. Thanks ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: spi: add loongson spi 2023-06-01 9:51 ` zhuyinbo @ 2023-06-01 15:30 ` Krzysztof Kozlowski 2023-06-02 6:46 ` zhuyinbo 0 siblings, 1 reply; 27+ messages in thread From: Krzysztof Kozlowski @ 2023-06-01 15:30 UTC (permalink / raw) To: zhuyinbo, Conor Dooley Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel On 01/06/2023 11:51, zhuyinbo wrote: >>> Yes, it is make sense as it can reduce the workload of the community. >>> For the Loongson platform, the versions of spi peripherals are almost >>> the same, except for a few or individual SoCs. And we have also >>> discussed compatible internally, and we tend to define it this way. >> >> So you have chosen different path than what's clearly recommended by >> community, existing experience and documentation: >> >> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 >> >> Family names are not accepted as specific compatibles. Whenever they >> were accepted, it lead to problems. All the time. > > > Thank you for your documentation and advice and the Loongson platform > have loongson-2h (ls2h), loongson-2k (ls2k), loongson-2p (ls2p) or other > series SoC, which loongson-2 seems to be the family name you mentioned > and the "loongson,ls2k-spi" should be a speific compatible name. > >> >> https://lore.kernel.org/all/20220822181701.GA89665-robh@kernel.org/ >> https://lore.kernel.org/all/78651e07-6b3e-4243-8e1f-fcd1dfb3ffe1@www.fastmail.com/ >> https://lore.kernel.org/all/288f56ba9cfad46354203b7698babe91@walle.cc/ >> https://lore.kernel.org/all/106e443a-e765-51fe-b556-e4e7e2aa771c@linaro.org/ >> and many many more discussions. >> >> You should choose carefully, because we will keep NAK-ing adding >> properties to circumvent missing compatibles. > > > I have read the documention and patch link that you mentioned and it > seems to advice that We don't have wildcard names in the compatible > string and use wildcard names that will cause issue. and the compatible > "loongson,ls2k-spi" that wasn't a wildcard names, and if the loongson-2k > spi controller hardware upgraded or changed the I will use > "loongson,ls2k-spi-version" as a compatible, such as, > "loongson,ls2k-spi-v1.1", "loongson,ls2k-spi-v1.1a" or other. Versions? Why? They received a lot of comments in the past, let me just paste to avoid repeating the same: https://lore.kernel.org/all/20220926231238.GA3132756-robh@kernel.org/ (and many more discussions on devicetree mailing list) > >>> >>>> Or am I misunderstanding and all ls2k SoCs do work with this driver and >>>> you were talking about other, future products? >>> >>> Actually, in 2k500 has one special type spi was only one cs and their's >>> register definition was different from common type spi thus this driver >>> doesn't support but this driver can support another common type spi in >>> 2k500. for this special type spi I will add support as needed in the >>> future. >> >> Bindings are for hardware, not driver. What does your driver support or >> does not, matters less. > > > okay, I got it, and the loongson spi bindings was for loongson spi > controller hardware. if the spi controller hardware not changed in > different ls2k SoC and the spi compatible should be same thus loongson > spi compatible seems to be adhere to the bindings aggrement. Specific compatible - yes. Unspecific - not, because you disregard the clear message in the guideline. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: spi: add loongson spi 2023-06-01 15:30 ` Krzysztof Kozlowski @ 2023-06-02 6:46 ` zhuyinbo 0 siblings, 0 replies; 27+ messages in thread From: zhuyinbo @ 2023-06-02 6:46 UTC (permalink / raw) To: Krzysztof Kozlowski, Conor Dooley Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/6/1 下午11:30, Krzysztof Kozlowski 写道: > On 01/06/2023 11:51, zhuyinbo wrote: >>>> Yes, it is make sense as it can reduce the workload of the community. >>>> For the Loongson platform, the versions of spi peripherals are almost >>>> the same, except for a few or individual SoCs. And we have also >>>> discussed compatible internally, and we tend to define it this way. >>> >>> So you have chosen different path than what's clearly recommended by >>> community, existing experience and documentation: >>> >>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 >>> >>> Family names are not accepted as specific compatibles. Whenever they >>> were accepted, it lead to problems. All the time. >> >> >> Thank you for your documentation and advice and the Loongson platform >> have loongson-2h (ls2h), loongson-2k (ls2k), loongson-2p (ls2p) or other >> series SoC, which loongson-2 seems to be the family name you mentioned >> and the "loongson,ls2k-spi" should be a speific compatible name. >> >>> >>> https://lore.kernel.org/all/20220822181701.GA89665-robh@kernel.org/ >>> https://lore.kernel.org/all/78651e07-6b3e-4243-8e1f-fcd1dfb3ffe1@www.fastmail.com/ >>> https://lore.kernel.org/all/288f56ba9cfad46354203b7698babe91@walle.cc/ >>> https://lore.kernel.org/all/106e443a-e765-51fe-b556-e4e7e2aa771c@linaro.org/ >>> and many many more discussions. >>> >>> You should choose carefully, because we will keep NAK-ing adding >>> properties to circumvent missing compatibles. >> >> >> I have read the documention and patch link that you mentioned and it >> seems to advice that We don't have wildcard names in the compatible >> string and use wildcard names that will cause issue. and the compatible >> "loongson,ls2k-spi" that wasn't a wildcard names, and if the loongson-2k >> spi controller hardware upgraded or changed the I will use >> "loongson,ls2k-spi-version" as a compatible, such as, >> "loongson,ls2k-spi-v1.1", "loongson,ls2k-spi-v1.1a" or other. > > Versions? Why? They received a lot of comments in the past, let me just > paste to avoid repeating the same: > > https://lore.kernel.org/all/20220926231238.GA3132756-robh@kernel.org/ > > (and many more discussions on devicetree mailing list) > I didn't notice the following words earlier about compatible in documention and I will use "loongson,ls2k1000-spi" as ls2k1000 SoC spi compatible, which is a very specific compatible. "For sub-blocks/components of bigger device (e.g. SoC blocks) use rather device-based compatible (e.g. SoC-based compatible), instead of custom versioning of that component.For example use "vendor,soc1234-i2c" instead of "vendor,i2c-v2". >> >>>> >>>>> Or am I misunderstanding and all ls2k SoCs do work with this driver and >>>>> you were talking about other, future products? >>>> >>>> Actually, in 2k500 has one special type spi was only one cs and their's >>>> register definition was different from common type spi thus this driver >>>> doesn't support but this driver can support another common type spi in >>>> 2k500. for this special type spi I will add support as needed in the >>>> future. >>> >>> Bindings are for hardware, not driver. What does your driver support or >>> does not, matters less. >> >> >> okay, I got it, and the loongson spi bindings was for loongson spi >> controller hardware. if the spi controller hardware not changed in >> different ls2k SoC and the spi compatible should be same thus loongson >> spi compatible seems to be adhere to the bindings aggrement. > > Specific compatible - yes. Unspecific - not, because you disregard the > clear message in the guideline. okay, I got it. Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <69d355ff-90e1-09d2-d4ff-0d7dedc8addb@linaro.org>]
* Re: [PATCH v11 1/2] dt-bindings: spi: add loongson spi [not found] ` <69d355ff-90e1-09d2-d4ff-0d7dedc8addb@linaro.org> @ 2023-06-01 11:38 ` zhuyinbo 0 siblings, 0 replies; 27+ messages in thread From: zhuyinbo @ 2023-06-01 11:38 UTC (permalink / raw) To: Krzysztof Kozlowski, Conor Dooley Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/6/1 上午4:06, Krzysztof Kozlowski 写道: > On 25/05/2023 04:22, zhuyinbo wrote: >> >> >> 在 2023/5/24 下午6:29, Conor Dooley 写道: >>> On Wed, May 24, 2023 at 05:44:38PM +0800, zhuyinbo wrote: >>>> >>>> >>>> 在 2023/5/24 下午4:56, Conor Dooley 写道: >>>>> On Mon, May 22, 2023 at 03:10:29PM +0800, Yinbo Zhu wrote: >>>>>> Add the Loongson platform spi binding with DT schema format using >>>>>> json-schema. >>>>>> >>>>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> >>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>>>> --- >>>>>> .../bindings/spi/loongson,ls2k-spi.yaml | 41 +++++++++++++++++++ >>>>>> MAINTAINERS | 6 +++ >>>>>> 2 files changed, 47 insertions(+) >>>>>> create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >>>>>> new file mode 100644 >>>>>> index 000000000000..d0be6e5378d7 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml >>>>>> @@ -0,0 +1,41 @@ >>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>>>> +%YAML 1.2 >>>>>> +--- >>>>>> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml# >>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>>> + >>>>>> +title: Loongson SPI controller >>>>>> + >>>>>> +maintainers: >>>>>> + - Yinbo Zhu <zhuyinbo@loongson.cn> >>>>>> + >>>>>> +allOf: >>>>>> + - $ref: /schemas/spi/spi-controller.yaml# >>>>>> + >>>>>> +properties: >>>>>> + compatible: >>>>>> + enum: >>>>>> + - loongson,ls2k-spi >>>>> >>>>> I am sorry to jump in here at such a late stage with a (potentially) >>>>> trivial question. "ls2k" is the SoC family rather than a specific model >>>>> as far as I understand. >>>>> The answer is probably yes, but do all SoCs in the family have an >>>>> identical version of the IP? >>>> >>>> >>>> No, but the spi supported by this loongson spi driver are all the same >>>> identical version, and other type or verion spi will be supported as >>>> needed in the future. >>> >>> Does having a catch-all compatible make sense then when not all SoCs in >>> the ls2k family will actually be able to use this driver? >> >> >> Yes, it is make sense as it can reduce the workload of the community. > > I missed it - that's a great comment. Hm, I don't know... Reviewing > Loongson patches is quite a work, so I don't see here reduced workload. If we do not consider the differences in SPI hardware and consider the differences in SoC, it will result for each new a SoC adaptation, a new compatible patch needs to be submitted to the community but spi hardware was same and that will increase the workload of the community, It seems to be more appropriate that use same compatible when spi hardware was same and use different compatible when spi hardware was different. > > Please read existing guidelines: > https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst > > All of them. > okay, I got it. Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller 2023-05-22 7:10 [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi Yinbo Zhu 2023-05-22 7:10 ` [PATCH v11 1/2] dt-bindings: spi: add " Yinbo Zhu @ 2023-05-22 7:10 ` Yinbo Zhu 2023-05-23 12:54 ` andy.shevchenko 2023-05-22 10:34 ` [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi Mark Brown 2023-07-31 19:57 ` Mark Brown 3 siblings, 1 reply; 27+ messages in thread From: Yinbo Zhu @ 2023-05-22 7:10 UTC (permalink / raw) To: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Yinbo Zhu This bus driver supports the Loongson SPI hardware controller in the Loongson platforms and supports to use DTS and PCI framework to register SPI device resources. Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> --- MAINTAINERS | 4 + drivers/spi/Kconfig | 26 +++ drivers/spi/Makefile | 3 + drivers/spi/spi-loongson-core.c | 279 ++++++++++++++++++++++++++++++++ drivers/spi/spi-loongson-pci.c | 61 +++++++ drivers/spi/spi-loongson-plat.c | 46 ++++++ drivers/spi/spi-loongson.h | 47 ++++++ 7 files changed, 466 insertions(+) create mode 100644 drivers/spi/spi-loongson-core.c create mode 100644 drivers/spi/spi-loongson-pci.c create mode 100644 drivers/spi/spi-loongson-plat.c create mode 100644 drivers/spi/spi-loongson.h diff --git a/MAINTAINERS b/MAINTAINERS index e49c04c53c99..fd63c5a1c886 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12206,6 +12206,10 @@ M: Yinbo Zhu <zhuyinbo@loongson.cn> L: linux-spi@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml +F: drivers/spi/spi-loongson-core.c +F: drivers/spi/spi-loongson-pci.c +F: drivers/spi/spi-loongson-plat.c +F: drivers/spi/spi-loongson.h LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) M: Sathya Prakash <sathya.prakash@broadcom.com> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 3de2ebe8294a..6b953904792e 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -516,6 +516,32 @@ config SPI_LM70_LLP which interfaces to an LM70 temperature sensor using a parallel port. +config SPI_LOONGSON_CORE + tristate + depends on LOONGARCH || COMPILE_TEST + +config SPI_LOONGSON_PCI + tristate "Loongson SPI Controller PCI Driver Support" + select SPI_LOONGSON_CORE + depends on PCI && (LOONGARCH || COMPILE_TEST) + help + This bus driver supports the Loongson SPI hardware controller in + the Loongson platforms and supports to use PCI framework to + register SPI device resources. + Say Y or M here if you want to use the SPI controller on + Loongson platform. + +config SPI_LOONGSON_PLATFORM + tristate "Loongson SPI Controller Platform Driver Support" + select SPI_LOONGSON_CORE + depends on OF && (LOONGARCH || COMPILE_TEST) + help + This bus driver supports the Loongson SPI hardware controller in + the Loongson platforms and supports to use DTS framework to + register SPI device resources. + Say Y or M here if you want to use the SPI controller on + Loongson platform. + config SPI_LP8841_RTC tristate "ICP DAS LP-8841 SPI Controller for RTC" depends on MACH_PXA27X_DT || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 28c4817a8a74..3e933064d237 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -71,6 +71,9 @@ obj-$(CONFIG_SPI_INTEL_PLATFORM) += spi-intel-platform.o obj-$(CONFIG_SPI_LANTIQ_SSC) += spi-lantiq-ssc.o obj-$(CONFIG_SPI_JCORE) += spi-jcore.o obj-$(CONFIG_SPI_LM70_LLP) += spi-lm70llp.o +obj-$(CONFIG_SPI_LOONGSON_CORE) += spi-loongson-core.o +obj-$(CONFIG_SPI_LOONGSON_PCI) += spi-loongson-pci.o +obj-$(CONFIG_SPI_LOONGSON_PLATFORM) += spi-loongson-plat.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 diff --git a/drivers/spi/spi-loongson-core.c b/drivers/spi/spi-loongson-core.c new file mode 100644 index 000000000000..a556c60155d6 --- /dev/null +++ b/drivers/spi/spi-loongson-core.c @@ -0,0 +1,279 @@ +// SPDX-License-Identifier: GPL-2.0+ +// Loongson SPI Support +// Copyright (C) 2023 Loongson Technology Corporation Limited + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/kernel.h> +#include <linux/module.h> + +#include "spi-loongson.h" + +static inline void loongson_spi_write_reg(struct loongson_spi *spi, unsigned char reg, + unsigned char data) +{ + writeb(data, spi->base + reg); +} + +static inline char loongson_spi_read_reg(struct loongson_spi *spi, unsigned char reg) +{ + return readb(spi->base + reg); +} + +static void loongson_spi_set_cs(struct spi_device *spi, bool val) +{ + int cs; + struct loongson_spi *loongson_spi = spi_master_get_devdata(spi->master); + + cs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG) + & ~(0x11 << spi_get_chipselect(spi, 0)); + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG, + (val ? (0x11 << spi_get_chipselect(spi, 0)) : + (0x1 << spi_get_chipselect(spi, 0))) | cs); +} + +static void loongson_spi_set_clk(struct loongson_spi *loongson_spi, unsigned int hz) +{ + unsigned char val; + unsigned int div, div_tmp; + const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11}; + + div = clamp_val(DIV_ROUND_UP_ULL(loongson_spi->clk_rate, hz), 2, 4096); + div_tmp = rdiv[fls(div - 1)]; + loongson_spi->spcr = (div_tmp & GENMASK(1, 0)) >> 0; + loongson_spi->sper = (div_tmp & GENMASK(3, 2)) >> 2; + val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG); + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, (val & ~3) | + loongson_spi->spcr); + val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG); + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, (val & ~3) | + loongson_spi->sper); + loongson_spi->hz = hz; +} + +static void loongson_spi_set_mode(struct loongson_spi *loongson_spi, + struct spi_device *spi) +{ + unsigned char val; + + val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG); + val &= ~(LOONGSON_SPI_SPCR_CPOL | LOONGSON_SPI_SPCR_CPHA); + if (spi->mode & SPI_CPOL) + val |= LOONGSON_SPI_SPCR_CPOL; + if (spi->mode & SPI_CPHA) + val |= LOONGSON_SPI_SPCR_CPHA; + + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, val); + loongson_spi->mode |= spi->mode; +} + +static int loongson_spi_update_state(struct loongson_spi *loongson_spi, + struct spi_device *spi, struct spi_transfer *t) +{ + unsigned int hz; + + if (t) + hz = t->speed_hz; + + if (hz && loongson_spi->hz != hz) + loongson_spi_set_clk(loongson_spi, hz); + + if ((spi->mode ^ loongson_spi->mode) & SPI_MODE_X_MASK) + loongson_spi_set_mode(loongson_spi, spi); + + return 0; +} + +static int loongson_spi_setup(struct spi_device *spi) +{ + struct loongson_spi *loongson_spi; + + loongson_spi = spi_master_get_devdata(spi->master); + if (spi->bits_per_word % 8) + return -EINVAL; + + if (spi_get_chipselect(spi, 0) >= spi->master->num_chipselect) + return -EINVAL; + + loongson_spi->hz = 0; + loongson_spi_set_cs(spi, 1); + + return 0; +} + +static int loongson_spi_write_read_8bit(struct spi_device *spi, const u8 **tx_buf, + u8 **rx_buf, unsigned int num) +{ + struct loongson_spi *loongson_spi = spi_master_get_devdata(spi->master); + + if (tx_buf && *tx_buf) + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, *((*tx_buf)++)); + else + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, 0); + readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr, + (loongson_spi->spsr & 0x1) != 1, 1, MSEC_PER_SEC); + + if (rx_buf && *rx_buf) + *(*rx_buf)++ = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG); + else + loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG); + + return 0; +} + +static unsigned int loongson_spi_write_read(struct spi_device *spi, struct spi_transfer *xfer) +{ + unsigned int count; + const u8 *tx = xfer->tx_buf; + u8 *rx = xfer->rx_buf; + + count = xfer->len; + + do { + if (loongson_spi_write_read_8bit(spi, &tx, &rx, count) < 0) + goto out; + count--; + } while (count); + +out: + return xfer->len - count; +} + +static int loongson_spi_prepare_message(struct spi_controller *ctlr, struct spi_message *m) +{ + struct loongson_spi *loongson_spi = spi_controller_get_devdata(ctlr); + + loongson_spi->para = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_PARA_REG); + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para & ~1); + + return 0; +} + +static int loongson_spi_transfer_one(struct spi_controller *ctrl, struct spi_device *spi, + struct spi_transfer *xfer) +{ + struct loongson_spi *loongson_spi = spi_master_get_devdata(spi->master); + + loongson_spi_update_state(loongson_spi, spi, xfer); + if (xfer->len) + xfer->len = loongson_spi_write_read(spi, xfer); + + return 0; +} + +static int loongson_spi_unprepare_message(struct spi_controller *ctrl, struct spi_message *m) +{ + struct loongson_spi *loongson_spi = spi_controller_get_devdata(ctrl); + + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para); + + return 0; +} + +static void loongson_spi_reginit(struct loongson_spi *loongson_spi_dev) +{ + unsigned char val; + + val = loongson_spi_read_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG); + val &= ~LOONGSON_SPI_SPCR_SPE; + loongson_spi_write_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG, val); + + loongson_spi_write_reg(loongson_spi_dev, LOONGSON_SPI_SPSR_REG, + (LOONGSON_SPI_SPSR_SPIF | LOONGSON_SPI_SPSR_WCOL)); + + val = loongson_spi_read_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG); + val |= LOONGSON_SPI_SPCR_SPE; + loongson_spi_write_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG, val); +} + +int loongson_spi_init_master(struct device *dev, void __iomem *regs) +{ + struct spi_master *master; + struct loongson_spi *spi; + struct clk *clk; + + master = devm_spi_alloc_master(dev, sizeof(struct loongson_spi)); + if (master == NULL) + return -ENOMEM; + + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH; + master->setup = loongson_spi_setup; + master->prepare_message = loongson_spi_prepare_message; + master->transfer_one = loongson_spi_transfer_one; + master->unprepare_message = loongson_spi_unprepare_message; + master->set_cs = loongson_spi_set_cs; + master->num_chipselect = 4; + device_set_node(&master->dev, dev_fwnode(dev)); + dev_set_drvdata(dev, master); + + spi = spi_master_get_devdata(master); + spi->base = regs; + spi->master = master; + + clk = devm_clk_get_optional(dev, NULL); + if (!IS_ERR(clk)) + spi->clk_rate = clk_get_rate(clk); + else + return dev_err_probe(dev, PTR_ERR(clk), "unable to get clock\n"); + + loongson_spi_reginit(spi); + + spi->mode = 0; + + return devm_spi_register_master(dev, master); +} +EXPORT_SYMBOL_NS_GPL(loongson_spi_init_master, SPI_LOONGSON_CORE); + +static int __maybe_unused loongson_spi_suspend(struct device *dev) +{ + struct loongson_spi *loongson_spi; + struct spi_master *master; + + master = dev_get_drvdata(dev); + spi_master_suspend(master); + + loongson_spi = spi_master_get_devdata(master); + + loongson_spi->spcr = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG); + loongson_spi->sper = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG); + loongson_spi->spsr = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPSR_REG); + loongson_spi->para = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_PARA_REG); + loongson_spi->sfcs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG); + loongson_spi->timi = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_TIMI_REG); + + return 0; +} + +static int __maybe_unused loongson_spi_resume(struct device *dev) +{ + struct loongson_spi *loongson_spi; + struct spi_master *master; + + master = dev_get_drvdata(dev); + loongson_spi = spi_master_get_devdata(master); + + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, loongson_spi->spcr); + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, loongson_spi->sper); + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPSR_REG, loongson_spi->spsr); + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para); + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG, loongson_spi->sfcs); + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_TIMI_REG, loongson_spi->timi); + + spi_master_resume(master); + + return 0; +} + +const struct dev_pm_ops loongson_spi_dev_pm_ops = { + .suspend = loongson_spi_suspend, + .resume = loongson_spi_resume, +}; +EXPORT_SYMBOL_NS_GPL(loongson_spi_dev_pm_ops, SPI_LOONGSON_CORE); + +MODULE_DESCRIPTION("Loongson SPI core driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/spi/spi-loongson-pci.c b/drivers/spi/spi-loongson-pci.c new file mode 100644 index 000000000000..c351a689150a --- /dev/null +++ b/drivers/spi/spi-loongson-pci.c @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: GPL-2.0+ +// PCI interface driver for Loongson SPI Support +// Copyright (C) 2023 Loongson Technology Corporation Limited + +#include <linux/pci.h> + +#include "spi-loongson.h" + +static int loongson_spi_pci_register(struct pci_dev *pdev, + const struct pci_device_id *ent) +{ + int ret; + void __iomem *reg_base; + struct device *dev = &pdev->dev; + int pci_bar = 0; + + ret = pcim_enable_device(pdev); + if (ret < 0) + return dev_err_probe(dev, ret, "cannot enable pci device\n"); + + ret = pcim_iomap_regions(pdev, BIT(pci_bar), pci_name(pdev)); + if (ret) + return dev_err_probe(dev, ret, "failed to request and remap memory\n"); + + reg_base = pcim_iomap_table(pdev)[pci_bar]; + + ret = loongson_spi_init_master(dev, reg_base); + if (ret) + return dev_err_probe(dev, ret, "failed to initialize master\n"); + + return 0; +} + +static void loongson_spi_pci_unregister(struct pci_dev *pdev) +{ + pcim_iounmap_regions(pdev, BIT(0)); + pci_disable_device(pdev); +} + +static struct pci_device_id loongson_spi_devices[] = { + { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a0b) }, + { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a1b) }, + { }, +}; +MODULE_DEVICE_TABLE(pci, loongson_spi_devices); + +static struct pci_driver loongson_spi_pci_driver = { + .name = "loongson-spi-pci", + .id_table = loongson_spi_devices, + .probe = loongson_spi_pci_register, + .remove = loongson_spi_pci_unregister, + .driver = { + .bus = &pci_bus_type, + .pm = &loongson_spi_dev_pm_ops, + }, +}; +module_pci_driver(loongson_spi_pci_driver); + +MODULE_DESCRIPTION("Loongson spi pci driver"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS(SPI_LOONGSON_CORE); diff --git a/drivers/spi/spi-loongson-plat.c b/drivers/spi/spi-loongson-plat.c new file mode 100644 index 000000000000..2e0388d84044 --- /dev/null +++ b/drivers/spi/spi-loongson-plat.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-2.0+ +// Platform driver for Loongson SPI Support +// Copyright (C) 2023 Loongson Technology Corporation Limited + +#include <linux/of.h> +#include <linux/platform_device.h> + +#include "spi-loongson.h" + +static int loongson_spi_platform_probe(struct platform_device *pdev) +{ + int ret; + void __iomem *reg_base; + struct device *dev = &pdev->dev; + + reg_base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(reg_base)) + return PTR_ERR(reg_base); + + ret = loongson_spi_init_master(dev, reg_base); + if (ret) + return dev_err_probe(dev, ret, "failed to initialize master\n"); + + return ret; +} + +static const struct of_device_id loongson_spi_id_table[] = { + { .compatible = "loongson,ls2k-spi" }, + { } +}; +MODULE_DEVICE_TABLE(of, loongson_spi_id_table); + +static struct platform_driver loongson_spi_plat_driver = { + .probe = loongson_spi_platform_probe, + .driver = { + .name = "loongson-spi", + .bus = &platform_bus_type, + .pm = &loongson_spi_dev_pm_ops, + .of_match_table = loongson_spi_id_table, + }, +}; +module_platform_driver(loongson_spi_plat_driver); + +MODULE_DESCRIPTION("Loongson spi platform driver"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS(SPI_LOONGSON_CORE); diff --git a/drivers/spi/spi-loongson.h b/drivers/spi/spi-loongson.h new file mode 100644 index 000000000000..5dca9750efa3 --- /dev/null +++ b/drivers/spi/spi-loongson.h @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Header File for Loongson SPI Driver. */ +/* Copyright (C) 2023 Loongson Technology Corporation Limited */ + +#ifndef __LINUX_SPI_LOONGSON_H +#define __LINUX_SPI_LOONGSON_H + +#include <linux/bits.h> +#include <linux/device.h> +#include <linux/pm.h> +#include <linux/spi/spi.h> +#include <linux/types.h> + +#define LOONGSON_SPI_SPCR_REG 0x00 +#define LOONGSON_SPI_SPSR_REG 0x01 +#define LOONGSON_SPI_FIFO_REG 0x02 +#define LOONGSON_SPI_SPER_REG 0x03 +#define LOONGSON_SPI_PARA_REG 0x04 +#define LOONGSON_SPI_SFCS_REG 0x05 +#define LOONGSON_SPI_TIMI_REG 0x06 + +/* Bits definition for Loongson SPI register */ +#define LOONGSON_SPI_PARA_MEM_EN BIT(0) +#define LOONGSON_SPI_SPCR_CPHA BIT(2) +#define LOONGSON_SPI_SPCR_CPOL BIT(3) +#define LOONGSON_SPI_SPCR_SPE BIT(6) +#define LOONGSON_SPI_SPSR_WCOL BIT(6) +#define LOONGSON_SPI_SPSR_SPIF BIT(7) + +struct loongson_spi { + struct spi_master *master; + void __iomem *base; + int cs_active; + unsigned int hz; + unsigned char spcr; + unsigned char sper; + unsigned char spsr; + unsigned char para; + unsigned char sfcs; + unsigned char timi; + unsigned int mode; + u64 clk_rate; +}; + +int loongson_spi_init_master(struct device *dev, void __iomem *reg); +extern const struct dev_pm_ops loongson_spi_dev_pm_ops; +#endif /* __LINUX_SPI_LOONGSON_H */ -- 2.20.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller 2023-05-22 7:10 ` [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller Yinbo Zhu @ 2023-05-23 12:54 ` andy.shevchenko 2023-05-24 7:52 ` zhuyinbo 0 siblings, 1 reply; 27+ messages in thread From: andy.shevchenko @ 2023-05-23 12:54 UTC (permalink / raw) To: Yinbo Zhu Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel Mon, May 22, 2023 at 03:10:30PM +0800, Yinbo Zhu kirjoitti: > This bus driver supports the Loongson SPI hardware controller in the > Loongson platforms and supports to use DTS and PCI framework to > register SPI device resources. It's polite to add reviewers of the previous versions to the Cc list. ... > +static void loongson_spi_set_clk(struct loongson_spi *loongson_spi, unsigned int hz) > +{ > + unsigned char val; > + unsigned int div, div_tmp; > + const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11}; static? > + > + div = clamp_val(DIV_ROUND_UP_ULL(loongson_spi->clk_rate, hz), 2, 4096); > + div_tmp = rdiv[fls(div - 1)]; > + loongson_spi->spcr = (div_tmp & GENMASK(1, 0)) >> 0; > + loongson_spi->sper = (div_tmp & GENMASK(3, 2)) >> 2; > + val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG); > + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, (val & ~3) | > + loongson_spi->spcr); > + val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG); > + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, (val & ~3) | > + loongson_spi->sper); > + loongson_spi->hz = hz; > +} ... > +static int loongson_spi_update_state(struct loongson_spi *loongson_spi, > + struct spi_device *spi, struct spi_transfer *t) > +{ > + unsigned int hz; > + > + if (t) > + hz = t->speed_hz; And if t is NULL? hz will be uninitialized. Don't you get a compiler warning? (Always test your code with `make W=1 ...`) > + if (hz && loongson_spi->hz != hz) > + loongson_spi_set_clk(loongson_spi, hz); > + > + if ((spi->mode ^ loongson_spi->mode) & SPI_MODE_X_MASK) > + loongson_spi_set_mode(loongson_spi, spi); > + > + return 0; > +} ... > + readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr, > + (loongson_spi->spsr & 0x1) != 1, 1, MSEC_PER_SEC); Wouldn't be better to use ' == 0' in the conditional? Or if you think your approach is better (to show the exact expectation) the definition of the bit 0 might help #define LOONGSON_... BIT(0) readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr, (loongson_spi->spsr & LOONGSON_...) != LOONGSON_..., 1, MSEC_PER_SEC); ... > + do { > + if (loongson_spi_write_read_8bit(spi, &tx, &rx, count) < 0) > + goto out; break; > + count--; > + } while (count); } while (--count); ? > +out: > + return xfer->len - count; Shouldn't you return an error code if the write failed? ... > + master = devm_spi_alloc_master(dev, sizeof(struct loongson_spi)); > + if (master == NULL) if (!master) > + return -ENOMEM; Why do you use deprecated naming? Can you use spi_controller* instead of spi_master* in all cases? ... > + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH; = SPI_MODE_X_MASK | SPI_CS_HIGH; ... > + clk = devm_clk_get_optional(dev, NULL); > + if (!IS_ERR(clk)) > + spi->clk_rate = clk_get_rate(clk); > + else Redundant. Just check for the error first as it's very usual pattern in the Linux kernel. > + return dev_err_probe(dev, PTR_ERR(clk), "unable to get clock\n"); ... > +static void loongson_spi_pci_unregister(struct pci_dev *pdev) > +{ > + pcim_iounmap_regions(pdev, BIT(0)); Not needed due to 'm' in the API name, which means "managed". > + pci_disable_device(pdev); This is simply wrong. We don't do explicit clean up for managed resources. > +} That said, drop the ->remove() completely. ... > +static struct pci_device_id loongson_spi_devices[] = { > + { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a0b) }, > + { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a1b) }, > + { }, No comma for the terminator entry. It's by definition "terminating" something. > +}; ... > +#include <linux/of.h> There is no user of this header. Please, replace with what actually is being used (presumably mod_devicetable.h and maybe others). > +#include <linux/platform_device.h> > + > +#include "spi-loongson.h" > + > +static int loongson_spi_platform_probe(struct platform_device *pdev) > +{ > + int ret; > + void __iomem *reg_base; > + struct device *dev = &pdev->dev; > + > + reg_base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(reg_base)) > + return PTR_ERR(reg_base); > + > + ret = loongson_spi_init_master(dev, reg_base); > + if (ret) > + return dev_err_probe(dev, ret, "failed to initialize master\n"); > + > + return ret; return 0; > +} ... > +#ifndef __LINUX_SPI_LOONGSON_H > +#define __LINUX_SPI_LOONGSON_H > + > +#include <linux/bits.h> > +#include <linux/device.h> This header is not used. > +#include <linux/pm.h> > +#include <linux/spi/spi.h> This neither. > +#include <linux/types.h> For them use forward declarations struct device; struct spi_controller; The rest of the inclusions is correct. ... > +#define LOONGSON_SPI_SPCR_REG 0x00 > +#define LOONGSON_SPI_SPSR_REG 0x01 > +#define LOONGSON_SPI_FIFO_REG 0x02 > +#define LOONGSON_SPI_SPER_REG 0x03 > +#define LOONGSON_SPI_PARA_REG 0x04 > +#define LOONGSON_SPI_SFCS_REG 0x05 > +#define LOONGSON_SPI_TIMI_REG 0x06 Where is this used outside of the main driver? > +/* Bits definition for Loongson SPI register */ > +#define LOONGSON_SPI_PARA_MEM_EN BIT(0) > +#define LOONGSON_SPI_SPCR_CPHA BIT(2) > +#define LOONGSON_SPI_SPCR_CPOL BIT(3) > +#define LOONGSON_SPI_SPCR_SPE BIT(6) > +#define LOONGSON_SPI_SPSR_WCOL BIT(6) > +#define LOONGSON_SPI_SPSR_SPIF BIT(7) > + > +struct loongson_spi { > + struct spi_master *master; > + void __iomem *base; > + int cs_active; > + unsigned int hz; > + unsigned char spcr; > + unsigned char sper; > + unsigned char spsr; > + unsigned char para; > + unsigned char sfcs; > + unsigned char timi; > + unsigned int mode; > + u64 clk_rate; > +}; > + > +int loongson_spi_init_master(struct device *dev, void __iomem *reg); > +extern const struct dev_pm_ops loongson_spi_dev_pm_ops; > +#endif /* __LINUX_SPI_LOONGSON_H */ -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller 2023-05-23 12:54 ` andy.shevchenko @ 2023-05-24 7:52 ` zhuyinbo 2023-05-24 8:42 ` Andy Shevchenko 0 siblings, 1 reply; 27+ messages in thread From: zhuyinbo @ 2023-05-24 7:52 UTC (permalink / raw) To: andy.shevchenko Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/5/23 下午8:54, andy.shevchenko@gmail.com 写道: > Mon, May 22, 2023 at 03:10:30PM +0800, Yinbo Zhu kirjoitti: >> This bus driver supports the Loongson SPI hardware controller in the >> Loongson platforms and supports to use DTS and PCI framework to >> register SPI device resources. > > It's polite to add reviewers of the previous versions to the Cc list. okay, I got it. > > ... > >> +static void loongson_spi_set_clk(struct loongson_spi *loongson_spi, unsigned int hz) >> +{ >> + unsigned char val; >> + unsigned int div, div_tmp; > >> + const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11}; > > static? okay, I will define "static const char rdiv". > >> + >> + div = clamp_val(DIV_ROUND_UP_ULL(loongson_spi->clk_rate, hz), 2, 4096); >> + div_tmp = rdiv[fls(div - 1)]; >> + loongson_spi->spcr = (div_tmp & GENMASK(1, 0)) >> 0; >> + loongson_spi->sper = (div_tmp & GENMASK(3, 2)) >> 2; >> + val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG); >> + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, (val & ~3) | >> + loongson_spi->spcr); >> + val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG); >> + loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, (val & ~3) | >> + loongson_spi->sper); >> + loongson_spi->hz = hz; >> +} > > ... > >> +static int loongson_spi_update_state(struct loongson_spi *loongson_spi, >> + struct spi_device *spi, struct spi_transfer *t) >> +{ >> + unsigned int hz; >> + >> + if (t) >> + hz = t->speed_hz; > > And if t is NULL? hz will be uninitialized. Don't you get a compiler warning? > (Always test your code with `make W=1 ...`) I alwasy use `make W=1` and I don't find any warnig, but that what you said was right and I will initial hz. > >> + if (hz && loongson_spi->hz != hz) >> + loongson_spi_set_clk(loongson_spi, hz); >> + >> + if ((spi->mode ^ loongson_spi->mode) & SPI_MODE_X_MASK) >> + loongson_spi_set_mode(loongson_spi, spi); >> + >> + return 0; >> +} > > ... > >> + readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr, >> + (loongson_spi->spsr & 0x1) != 1, 1, MSEC_PER_SEC); > > Wouldn't be better to use ' == 0' in the conditional? Or if you think your > approach is better (to show the exact expectation) the definition of the bit 0 > might help > > #define LOONGSON_... BIT(0) okay, I got it. > > > readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr, > (loongson_spi->spsr & LOONGSON_...) != LOONGSON_..., > 1, MSEC_PER_SEC); > > ... > >> + do { >> + if (loongson_spi_write_read_8bit(spi, &tx, &rx, count) < 0) > >> + goto out; > > break; > >> + count--; >> + } while (count); > > } while (--count); > > ? okay, I got it. > >> +out: >> + return xfer->len - count; > > Shouldn't you return an error code if the write failed? okay, I got it. I will add a error code for return when write failed. > > ... > >> + master = devm_spi_alloc_master(dev, sizeof(struct loongson_spi)); > >> + if (master == NULL) > > if (!master) > >> + return -ENOMEM; > > Why do you use deprecated naming? Can you use spi_controller* instead of > spi_master* in all cases? It seems was a personal code style issue and I don't find the differences between spi_controller and spi_master, Using spi_controller* is also acceptable to me and I will use spi_controller* instead of spi_master* in all cases. > > ... > >> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH; > > = SPI_MODE_X_MASK | SPI_CS_HIGH; > > ... > >> + clk = devm_clk_get_optional(dev, NULL); >> + if (!IS_ERR(clk)) >> + spi->clk_rate = clk_get_rate(clk); > >> + else > > Redundant. Just check for the error first as it's very usual pattern in the > Linux kernel. Like below ? clk = devm_clk_get_optional(dev, NULL); - if (!IS_ERR(clk)) - spi->clk_rate = clk_get_rate(clk); - else + if (IS_ERR(clk)) return dev_err_probe(dev, PTR_ERR(clk), "unable to get clock\n"); + spi->clk_rate = clk_get_rate(clk); loongson_spi_reginit(spi); > >> + return dev_err_probe(dev, PTR_ERR(clk), "unable to get clock\n"); > > ... > >> +static void loongson_spi_pci_unregister(struct pci_dev *pdev) >> +{ > >> + pcim_iounmap_regions(pdev, BIT(0)); > > Not needed due to 'm' in the API name, which means "managed". > >> + pci_disable_device(pdev); > > This is simply wrong. We don't do explicit clean up for managed resources. > >> +} > > That said, drop the ->remove() completely. okay, I will drop the ->remove() completely. > > ... > >> +static struct pci_device_id loongson_spi_devices[] = { >> + { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a0b) }, >> + { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a1b) }, >> + { }, > > No comma for the terminator entry. It's by definition "terminating" something. okay, I got it. > >> +}; > > ... > >> +#include <linux/of.h> > > There is no user of this header. Please, replace with what actually is being > used (presumably mod_devicetable.h and maybe others). > okay, I got it. >> +#include <linux/platform_device.h> >> + >> +#include "spi-loongson.h" >> + >> +static int loongson_spi_platform_probe(struct platform_device *pdev) >> +{ >> + int ret; >> + void __iomem *reg_base; >> + struct device *dev = &pdev->dev; >> + >> + reg_base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(reg_base)) >> + return PTR_ERR(reg_base); >> + >> + ret = loongson_spi_init_master(dev, reg_base); >> + if (ret) >> + return dev_err_probe(dev, ret, "failed to initialize master\n"); >> + >> + return ret; > > return 0; It seems was more appropriate that initialize ret then return ret. Do you think so ? > >> +} > > ... > >> +#ifndef __LINUX_SPI_LOONGSON_H >> +#define __LINUX_SPI_LOONGSON_H >> + >> +#include <linux/bits.h> > >> +#include <linux/device.h> > > This header is not used. okay, I got it. > >> +#include <linux/pm.h> > >> +#include <linux/spi/spi.h> > > This neither. That other .c file seems to need it and I will move it to other .c code file. > >> +#include <linux/types.h> > > > For them use forward declarations > > struct device; > struct spi_controller; > > The rest of the inclusions is correct. okay, I got it. > > ... > >> +#define LOONGSON_SPI_SPCR_REG 0x00 >> +#define LOONGSON_SPI_SPSR_REG 0x01 >> +#define LOONGSON_SPI_FIFO_REG 0x02 >> +#define LOONGSON_SPI_SPER_REG 0x03 >> +#define LOONGSON_SPI_PARA_REG 0x04 >> +#define LOONGSON_SPI_SFCS_REG 0x05 >> +#define LOONGSON_SPI_TIMI_REG 0x06 > > Where is this used outside of the main driver? These definitions are only used in core.c > >> +/* Bits definition for Loongson SPI register */ >> +#define LOONGSON_SPI_PARA_MEM_EN BIT(0) >> +#define LOONGSON_SPI_SPCR_CPHA BIT(2) >> +#define LOONGSON_SPI_SPCR_CPOL BIT(3) >> +#define LOONGSON_SPI_SPCR_SPE BIT(6) >> +#define LOONGSON_SPI_SPSR_WCOL BIT(6) >> +#define LOONGSON_SPI_SPSR_SPIF BIT(7) >> + >> +struct loongson_spi { >> + struct spi_master *master; >> + void __iomem *base; >> + int cs_active; >> + unsigned int hz; >> + unsigned char spcr; >> + unsigned char sper; >> + unsigned char spsr; >> + unsigned char para; >> + unsigned char sfcs; >> + unsigned char timi; >> + unsigned int mode; >> + u64 clk_rate; >> +}; >> + >> +int loongson_spi_init_master(struct device *dev, void __iomem *reg); >> +extern const struct dev_pm_ops loongson_spi_dev_pm_ops; >> +#endif /* __LINUX_SPI_LOONGSON_H */ > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller 2023-05-24 7:52 ` zhuyinbo @ 2023-05-24 8:42 ` Andy Shevchenko 2023-05-24 10:19 ` Mark Brown 2023-05-25 3:34 ` zhuyinbo 0 siblings, 2 replies; 27+ messages in thread From: Andy Shevchenko @ 2023-05-24 8:42 UTC (permalink / raw) To: zhuyinbo Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel On Wed, May 24, 2023 at 10:52 AM zhuyinbo <zhuyinbo@loongson.cn> wrote: > 在 2023/5/23 下午8:54, andy.shevchenko@gmail.com 写道: > > Mon, May 22, 2023 at 03:10:30PM +0800, Yinbo Zhu kirjoitti: ... > >> +static int loongson_spi_update_state(struct loongson_spi *loongson_spi, > >> + struct spi_device *spi, struct spi_transfer *t) > >> +{ > >> + unsigned int hz; > >> + > >> + if (t) > >> + hz = t->speed_hz; > > > > And if t is NULL? hz will be uninitialized. Don't you get a compiler warning? > > (Always test your code with `make W=1 ...`) > > I always use `make W=1` and I don't find any warning, but that what you > said was right and I will initial hz. Note, if hz == 0 when t == NULL, you can unify that check with the below. > >> + if (hz && loongson_spi->hz != hz) Something like if (t && _spi->hz != t->speed_hz) ...(..., t->speed_hz); In such a case you won't need a temporary variable. > >> + loongson_spi_set_clk(loongson_spi, hz); > >> + > >> + if ((spi->mode ^ loongson_spi->mode) & SPI_MODE_X_MASK) > >> + loongson_spi_set_mode(loongson_spi, spi); > >> + > >> + return 0; > >> +} ... > > Why do you use deprecated naming? Can you use spi_controller* instead of > > spi_master* in all cases? > > It seems was a personal code style issue and I don't find the > differences between spi_controller and spi_master, Using spi_controller* > is also acceptable to me and I will use spi_controller* instead of > spi_master* in all cases. Read this section (#4) in full https://kernel.org/doc/html/latest/process/coding-style.html#naming ... > >> + clk = devm_clk_get_optional(dev, NULL); > >> + if (!IS_ERR(clk)) > >> + spi->clk_rate = clk_get_rate(clk); > > > >> + else > > > > Redundant. Just check for the error first as it's very usual pattern in the > > Linux kernel. > > Like below ? > > clk = devm_clk_get_optional(dev, NULL); > - if (!IS_ERR(clk)) > - spi->clk_rate = clk_get_rate(clk); > - else > + if (IS_ERR(clk)) > return dev_err_probe(dev, PTR_ERR(clk), "unable to get > clock\n"); > > + spi->clk_rate = clk_get_rate(clk); Yes. > loongson_spi_reginit(spi); > >> + return dev_err_probe(dev, PTR_ERR(clk), "unable to get clock\n"); ... > >> + ret = loongson_spi_init_master(dev, reg_base); > >> + if (ret) > >> + return dev_err_probe(dev, ret, "failed to initialize master\n"); > >> + > >> + return ret; > > > > return 0; > > It seems was more appropriate that initialize ret then return ret. > Do you think so ? What do you mean and how does it help here? ... > >> +#include <linux/spi/spi.h> > > > > This neither. > > That other .c file seems to need it and I will move it to other .c > code file. Yes, please do. ... > >> +#define LOONGSON_SPI_SPCR_REG 0x00 > >> +#define LOONGSON_SPI_SPSR_REG 0x01 > >> +#define LOONGSON_SPI_FIFO_REG 0x02 > >> +#define LOONGSON_SPI_SPER_REG 0x03 > >> +#define LOONGSON_SPI_PARA_REG 0x04 > >> +#define LOONGSON_SPI_SFCS_REG 0x05 > >> +#define LOONGSON_SPI_TIMI_REG 0x06 > > > > Where is this used outside of the main driver? > > These definitions are only used in core.c Then the obvious question, why are they located in *.h? ... > >> +/* Bits definition for Loongson SPI register */ > >> +#define LOONGSON_SPI_PARA_MEM_EN BIT(0) > >> +#define LOONGSON_SPI_SPCR_CPHA BIT(2) > >> +#define LOONGSON_SPI_SPCR_CPOL BIT(3) > >> +#define LOONGSON_SPI_SPCR_SPE BIT(6) > >> +#define LOONGSON_SPI_SPSR_WCOL BIT(6) > >> +#define LOONGSON_SPI_SPSR_SPIF BIT(7) Similar question here. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller 2023-05-24 8:42 ` Andy Shevchenko @ 2023-05-24 10:19 ` Mark Brown 2023-05-25 3:34 ` zhuyinbo 1 sibling, 0 replies; 27+ messages in thread From: Mark Brown @ 2023-05-24 10:19 UTC (permalink / raw) To: Andy Shevchenko Cc: zhuyinbo, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel [-- Attachment #1: Type: text/plain, Size: 906 bytes --] On Wed, May 24, 2023 at 11:42:34AM +0300, Andy Shevchenko wrote: > On Wed, May 24, 2023 at 10:52 AM zhuyinbo <zhuyinbo@loongson.cn> wrote: > > >> +#define LOONGSON_SPI_SPCR_REG 0x00 > > >> +#define LOONGSON_SPI_SPSR_REG 0x01 > > >> +#define LOONGSON_SPI_FIFO_REG 0x02 > > >> +#define LOONGSON_SPI_SPER_REG 0x03 > > >> +#define LOONGSON_SPI_PARA_REG 0x04 > > >> +#define LOONGSON_SPI_SFCS_REG 0x05 > > >> +#define LOONGSON_SPI_TIMI_REG 0x06 > > > Where is this used outside of the main driver? > > These definitions are only used in core.c > Then the obvious question, why are they located in *.h? It's absolutely fine to put them in a header file, that's a perfectly normal way of writing code - it helps keep the driver a bit smaller by putting big piles of defines in a separate file, that can help make things a bit more manageable. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller 2023-05-24 8:42 ` Andy Shevchenko 2023-05-24 10:19 ` Mark Brown @ 2023-05-25 3:34 ` zhuyinbo 2023-05-25 9:16 ` Andy Shevchenko 1 sibling, 1 reply; 27+ messages in thread From: zhuyinbo @ 2023-05-25 3:34 UTC (permalink / raw) To: Andy Shevchenko Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/5/24 下午4:42, Andy Shevchenko 写道: > On Wed, May 24, 2023 at 10:52 AM zhuyinbo <zhuyinbo@loongson.cn> wrote: >> 在 2023/5/23 下午8:54, andy.shevchenko@gmail.com 写道: >>> Mon, May 22, 2023 at 03:10:30PM +0800, Yinbo Zhu kirjoitti: > > ... > >>>> +static int loongson_spi_update_state(struct loongson_spi *loongson_spi, >>>> + struct spi_device *spi, struct spi_transfer *t) >>>> +{ >>>> + unsigned int hz; >>>> + >>>> + if (t) >>>> + hz = t->speed_hz; >>> >>> And if t is NULL? hz will be uninitialized. Don't you get a compiler warning? >>> (Always test your code with `make W=1 ...`) >> >> I always use `make W=1` and I don't find any warning, but that what you >> said was right and I will initial hz. > > Note, if hz == 0 when t == NULL, you can unify that check with the below. > >>>> + if (hz && loongson_spi->hz != hz) > > Something like > > if (t && _spi->hz != t->speed_hz) > ...(..., t->speed_hz); > > In such a case you won't need a temporary variable. okay, I got it. > >>>> + loongson_spi_set_clk(loongson_spi, hz); >>>> + >>>> + if ((spi->mode ^ loongson_spi->mode) & SPI_MODE_X_MASK) >>>> + loongson_spi_set_mode(loongson_spi, spi); >>>> + >>>> + return 0; >>>> +} > > ... > >>> Why do you use deprecated naming? Can you use spi_controller* instead of >>> spi_master* in all cases? >> >> It seems was a personal code style issue and I don't find the >> differences between spi_controller and spi_master, Using spi_controller* >> is also acceptable to me and I will use spi_controller* instead of >> spi_master* in all cases. > > Read this section (#4) in full > https://kernel.org/doc/html/latest/process/coding-style.html#naming okay, I got it. > > ... > >>>> + clk = devm_clk_get_optional(dev, NULL); >>>> + if (!IS_ERR(clk)) >>>> + spi->clk_rate = clk_get_rate(clk); >>> >>>> + else >>> >>> Redundant. Just check for the error first as it's very usual pattern in the >>> Linux kernel. >> >> Like below ? >> >> clk = devm_clk_get_optional(dev, NULL); >> - if (!IS_ERR(clk)) >> - spi->clk_rate = clk_get_rate(clk); >> - else >> + if (IS_ERR(clk)) >> return dev_err_probe(dev, PTR_ERR(clk), "unable to get >> clock\n"); >> >> + spi->clk_rate = clk_get_rate(clk); > > Yes. okay, I got it. > >> loongson_spi_reginit(spi); > >>>> + return dev_err_probe(dev, PTR_ERR(clk), "unable to get clock\n"); > > ... > >>>> + ret = loongson_spi_init_master(dev, reg_base); >>>> + if (ret) >>>> + return dev_err_probe(dev, ret, "failed to initialize master\n"); >>>> + >>>> + return ret; >>> >>> return 0; >> >> It seems was more appropriate that initialize ret then return ret. >> Do you think so ? > > What do you mean and how does it help here? I'm sorry, I was wrong before and the ret varible seems not to be initialized and it always record the return value for loongson_spi_init_master. It seems was appropriate that use "return ret" and I don't got your point that in probe for use "return 0" > > > ... > >>>> +#include <linux/spi/spi.h> >>> >>> This neither. >> >> That other .c file seems to need it and I will move it to other .c >> code file. > > Yes, please do. okay, I got it. Thanks, Yinbo ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller 2023-05-25 3:34 ` zhuyinbo @ 2023-05-25 9:16 ` Andy Shevchenko 2023-05-25 9:28 ` zhuyinbo 0 siblings, 1 reply; 27+ messages in thread From: Andy Shevchenko @ 2023-05-25 9:16 UTC (permalink / raw) To: zhuyinbo Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel On Thu, May 25, 2023 at 6:34 AM zhuyinbo <zhuyinbo@loongson.cn> wrote: > 在 2023/5/24 下午4:42, Andy Shevchenko 写道: > > On Wed, May 24, 2023 at 10:52 AM zhuyinbo <zhuyinbo@loongson.cn> wrote: > >> 在 2023/5/23 下午8:54, andy.shevchenko@gmail.com 写道: > >>> Mon, May 22, 2023 at 03:10:30PM +0800, Yinbo Zhu kirjoitti: ... > >>>> + ret = loongson_spi_init_master(dev, reg_base); > >>>> + if (ret) > >>>> + return dev_err_probe(dev, ret, "failed to initialize master\n"); > >>>> + > >>>> + return ret; > >>> > >>> return 0; > >> > >> It seems was more appropriate that initialize ret then return ret. > >> Do you think so ? > > > > What do you mean and how does it help here? > > I'm sorry, I was wrong before and the ret varible seems not to be > initialized and it always record the return value for > loongson_spi_init_master. > > It seems was appropriate that use "return ret" and I don't got your > point that in probe for use "return 0" In the above excerpt you will return anything except 0 with return dev_err_probe(); line. Why do you still need to return ret; at the end of the function? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller 2023-05-25 9:16 ` Andy Shevchenko @ 2023-05-25 9:28 ` zhuyinbo 0 siblings, 0 replies; 27+ messages in thread From: zhuyinbo @ 2023-05-25 9:28 UTC (permalink / raw) To: Andy Shevchenko Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/5/25 下午5:16, Andy Shevchenko 写道: > On Thu, May 25, 2023 at 6:34 AM zhuyinbo <zhuyinbo@loongson.cn> wrote: >> 在 2023/5/24 下午4:42, Andy Shevchenko 写道: >>> On Wed, May 24, 2023 at 10:52 AM zhuyinbo <zhuyinbo@loongson.cn> wrote: >>>> 在 2023/5/23 下午8:54, andy.shevchenko@gmail.com 写道: >>>>> Mon, May 22, 2023 at 03:10:30PM +0800, Yinbo Zhu kirjoitti: > > ... > >>>>>> + ret = loongson_spi_init_master(dev, reg_base); >>>>>> + if (ret) >>>>>> + return dev_err_probe(dev, ret, "failed to initialize master\n"); >>>>>> + >>>>>> + return ret; >>>>> >>>>> return 0; >>>> >>>> It seems was more appropriate that initialize ret then return ret. >>>> Do you think so ? >>> >>> What do you mean and how does it help here? >> >> I'm sorry, I was wrong before and the ret varible seems not to be >> initialized and it always record the return value for >> loongson_spi_init_master. >> >> It seems was appropriate that use "return ret" and I don't got your >> point that in probe for use "return 0" > > In the above excerpt you will return anything except 0 with return > dev_err_probe(); line. Why do you still need to return ret; at the end > of the function? I'm sorry, I misread it and you are right and I will "return 0". Thanks, Yinbo. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi 2023-05-22 7:10 [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi Yinbo Zhu 2023-05-22 7:10 ` [PATCH v11 1/2] dt-bindings: spi: add " Yinbo Zhu 2023-05-22 7:10 ` [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller Yinbo Zhu @ 2023-05-22 10:34 ` Mark Brown 2023-05-22 11:44 ` zhuyinbo 2023-07-31 19:57 ` Mark Brown 3 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2023-05-22 10:34 UTC (permalink / raw) To: Yinbo Zhu Cc: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel [-- Attachment #1: Type: text/plain, Size: 356 bytes --] On Mon, May 22, 2023 at 03:10:28PM +0800, Yinbo Zhu wrote: > Loongson platform support spi hardware controller and this series patch > was to add spi driver and binding support. To repeat what I previously asked you *please* send patches against my current tree, this doesn't even apply cleanly against Linus' tree never mind any of the branches in mine. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi 2023-05-22 10:34 ` [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi Mark Brown @ 2023-05-22 11:44 ` zhuyinbo 2023-05-22 11:56 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: zhuyinbo @ 2023-05-22 11:44 UTC (permalink / raw) To: Mark Brown Cc: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/5/22 下午6:34, Mark Brown 写道: > On Mon, May 22, 2023 at 03:10:28PM +0800, Yinbo Zhu wrote: >> Loongson platform support spi hardware controller and this series patch >> was to add spi driver and binding support. > > To repeat what I previously asked you *please* send patches against my > current tree, this doesn't even apply cleanly against Linus' tree never > mind any of the branches in mine. Hi Mark, I was base on following tree and branch to apply my patch then to send it to upstrem: tree: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git branch: for-next The recently patch was as follows, It seems no issue for patch apply them, Maybe it is the 0/2 patch issue ? this 0/2 patch wasn't a valid patch and it need was skipped. 8f7b91d47211 spi: loongson: add bus driver for the loongson spi controller 193a72146430 dt-bindings: spi: add loongson spi b1f4091a9eff (spi/for-next) Merge remote-tracking branch 'spi/for-6.5' into spi-next 120e1aa2f2e6 (spi/for-6.5) spi: hisi-kunpeng: Fix error checking f2156989bf30 spi: cdns: Add compatible for AMD Pensando Elba SoC Thanks, Yinbo. > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi 2023-05-22 11:44 ` zhuyinbo @ 2023-05-22 11:56 ` Mark Brown 2023-05-22 13:07 ` zhuyinbo 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2023-05-22 11:56 UTC (permalink / raw) To: zhuyinbo Cc: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel [-- Attachment #1: Type: text/plain, Size: 413 bytes --] On Mon, May 22, 2023 at 07:44:49PM +0800, zhuyinbo wrote: > The recently patch was as follows, It seems no issue for patch apply > them, Maybe it is the 0/2 patch issue ? this 0/2 patch wasn't a valid > patch and it need was skipped. What's causing problem is that you patched MAINTAINERS in both patches but also used the wrong subject line for the first patch so I was having to fix it up by hand every time. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi 2023-05-22 11:56 ` Mark Brown @ 2023-05-22 13:07 ` zhuyinbo 2023-05-22 13:10 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: zhuyinbo @ 2023-05-22 13:07 UTC (permalink / raw) To: Mark Brown Cc: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/5/22 下午7:56, Mark Brown 写道: > On Mon, May 22, 2023 at 07:44:49PM +0800, zhuyinbo wrote: > >> The recently patch was as follows, It seems no issue for patch apply >> them, Maybe it is the 0/2 patch issue ? this 0/2 patch wasn't a valid >> patch and it need was skipped. > > What's causing problem is that you patched MAINTAINERS in both patches > but also used the wrong subject line for the first patch so I was having > to fix it up by hand every time. Hi Mark, I learn about what you said that bindings patch and spi driver change a same MAINTAINERS file, but It seems not cause apply fail if the patch series apply in order. I'm sorry, I don't understand the reason why my spi series patch apply failed, then I have a look about your spi ci tree and that what I need to do is just change the title of [1/2] patch like this in next version ? Correcting the title can solve the problem of patch series apply failure in your tree ? actually, I don't reproduce that apply faile issue in your current spi tree and for-next branch. spi: add loongson spi bindings Thanks Yinbo ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi 2023-05-22 13:07 ` zhuyinbo @ 2023-05-22 13:10 ` Mark Brown 2023-05-23 2:08 ` zhuyinbo 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2023-05-22 13:10 UTC (permalink / raw) To: zhuyinbo Cc: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel [-- Attachment #1: Type: text/plain, Size: 825 bytes --] On Mon, May 22, 2023 at 09:07:47PM +0800, zhuyinbo wrote: > 在 2023/5/22 下午7:56, Mark Brown 写道: > > What's causing problem is that you patched MAINTAINERS in both patches > > but also used the wrong subject line for the first patch so I was having > > to fix it up by hand every time. > spi series patch apply failed, then I have a look about your spi ci > tree and that what I need to do is just change the title of [1/2] patch > like this in next version ? Correcting the title can solve the problem > of patch series apply failure in your tree ? actually, I don't reproduce > that apply faile issue in your current spi tree and for-next branch. > spi: add loongson spi bindings That's a good title. The patches get reordered in the mailbox when I rewrite the title prior to applying them. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi 2023-05-22 13:10 ` Mark Brown @ 2023-05-23 2:08 ` zhuyinbo 2023-05-23 9:57 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: zhuyinbo @ 2023-05-23 2:08 UTC (permalink / raw) To: Mark Brown Cc: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/5/22 下午9:10, Mark Brown 写道: > On Mon, May 22, 2023 at 09:07:47PM +0800, zhuyinbo wrote: >> 在 2023/5/22 下午7:56, Mark Brown 写道: > >>> What's causing problem is that you patched MAINTAINERS in both patches >>> but also used the wrong subject line for the first patch so I was having >>> to fix it up by hand every time. > >> spi series patch apply failed, then I have a look about your spi ci >> tree and that what I need to do is just change the title of [1/2] patch >> like this in next version ? Correcting the title can solve the problem >> of patch series apply failure in your tree ? actually, I don't reproduce >> that apply faile issue in your current spi tree and for-next branch. > >> spi: add loongson spi bindings > > That's a good title. The patches get reordered in the mailbox when I > rewrite the title prior to applying them. okay, I got it. thanks! and I noticed my v11 patch already exists in your ci tree that contain the title change and I whether need send the v12 that for fix the binding patch title ? Thanks, Yinbo > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi 2023-05-23 2:08 ` zhuyinbo @ 2023-05-23 9:57 ` Mark Brown 2023-05-23 11:01 ` zhuyinbo 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2023-05-23 9:57 UTC (permalink / raw) To: zhuyinbo Cc: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel [-- Attachment #1: Type: text/plain, Size: 266 bytes --] On Tue, May 23, 2023 at 10:08:25AM +0800, zhuyinbo wrote: > okay, I got it. thanks! and I noticed my v11 patch already exists in > your ci tree that contain the title change and I whether need send the > v12 that for fix the binding patch title ? No, it's fine. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi 2023-05-23 9:57 ` Mark Brown @ 2023-05-23 11:01 ` zhuyinbo 0 siblings, 0 replies; 27+ messages in thread From: zhuyinbo @ 2023-05-23 11:01 UTC (permalink / raw) To: Mark Brown Cc: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo 在 2023/5/23 下午5:57, Mark Brown 写道: > On Tue, May 23, 2023 at 10:08:25AM +0800, zhuyinbo wrote: > >> okay, I got it. thanks! and I noticed my v11 patch already exists in >> your ci tree that contain the title change and I whether need send the >> v12 that for fix the binding patch title ? > > No, it's fine. okay, I got it. Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi 2023-05-22 7:10 [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi Yinbo Zhu ` (2 preceding siblings ...) 2023-05-22 10:34 ` [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi Mark Brown @ 2023-07-31 19:57 ` Mark Brown 3 siblings, 0 replies; 27+ messages in thread From: Mark Brown @ 2023-07-31 19:57 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel, Yinbo Zhu Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel On Mon, 22 May 2023 15:10:28 +0800, Yinbo Zhu wrote: > Loongson platform support spi hardware controller and this series patch > was to add spi driver and binding support. > > Change in v2: > 1. This [PATCH v2 1/2] dt-bindings patch need depend on clk patch: > https:// > lore.kernel.org/all/20230307115022.12846-1-zhuyinbo@loongson.cn/ > 2. Remove the clock-names in spi yaml file. > 3. Add "loongson,ls7a-spi" compatible in spi yaml file. > 4. Add an || COMPILE_TEST and drop && PCI then add some CONFIG_PCI > macro to limit some pci code. > 5. Make the spi driver top code comment block that use C++ style. > 6. Drop spi->max_speed_hz. > 7. Add a spin_lock for loongson_spi_setup. > 8. Add a timeout and cpu_relax() in loongson_spi_write_read_8bit. > 9. Add spi_transfer_one and drop transfer and rework entire spi > driver that include some necessary changes. > 10. Use module_init replace subsys_initcall. > 11. About PM interface that I don't find any issue so I don't add > any changes. > Change in v3: > 1. This [PATCH v3 1/2] dt-bindings patch need depend on clk patch: > https:// > lore.kernel.org/all/20230323025229.2971-1-zhuyinbo@loongson.cn/ > 2. Drop the unused blank line in loongson,ls-spi.yaml file. > 3. Replace clock minItems with clock maxItems in yaml file. > 4. Separate spi driver into platform module, pci module and core > module. > 5. Replace DIV_ROUND_UP with DIV_ROUND_UP_ULL to fix compile error > "undefined reference to `__aeabi_uldivmod'" and "__udivdi3 undefined" > that reported by test robot. > 6. Remove the spin lock. > 7. Clear the loongson_spi->hz and loongson_spi->mode in setup to fixup > the issue that multiple spi device transfer that maybe cause spi was > be misconfigured. > Change in v4: > 1. This [PATCH v4 1/2] dt-bindings patch need depend on clk patch: > https:// > lore.kernel.org/all/20230323025229.2971-1-zhuyinbo@loongson.cn/ > 2. Add "#include <linux/io.h>" in spi-loongson-core.c for fix the compile > issue which devm_ioremap no declaration. > 3. Add "EXPORT_SYMBOL_GPL(loongson_spi_dev_pm_ops)" in > spi-loongson-core.c for fix the compile issue which > loongson_spi_dev_pm_ops undefined. > Change in v5: > 1. Get rid of the clock patch's dependency and open-code the clock IDs. > 2. Fixup checkpatch issue that by installed ply and gitpython package > locally, but this series of patch's code doesn't have any change. > Change in v6: > 1. Remove the "#include <dt-bindings/clock/loongson,ls2k-clk.h>" in > yaml file. > Change in v7: > 1. Remove the "loongson,ls7a-spi" and change yaml file name as > "loongson,ls2k-spi.yaml". > 2. Use module_pci_driver and module_platform_driver to replace > module_init and module_exit. > 3. Drop ".owner = THIS_MODULE" in spi platform driver. > 4. Add devm_spi_alloc_master devm_spi_register_master to simplify code. > 5. Add pci_disable_device() in loongson_spi_pci_unregister. > Change in v8: > 1. Add reviewed-by information for spi bindings patch. > 2. Fixup the uncorrect spi yaml file path in MAINTAINERS file. > 3. Add spi_master_suspend and spi_master_resume in spi pm function. > Change in v9: > 1. Make spi_master_suspend go first in pm suspend. > Change in v10: > 1. Fix the compile issue about of_node_get and of_get_property no > declaration. > 2. set config SPI_LOONGSON_CORE invisible. > 3. Captial "spi" in commit log and Kconfig file. > 4. Write header files in alphabetical order. > 5. Use clamp_val, GENMASK() and BIT() in spi clock setting. > 6. Optimize clock and mode setting code. > 7. Use readb_poll_timeout in loongson_spi_write_read_8bit. > 8. Remove some useless dmesg print. > 9. Use device_set_node replace of_node_get. > 10. Use dev_err_probe in code. > 11. Use devm_clk_get_optional replace devm_clk_get. > 12. Remove SPI_NO_CS for drop 2k500 non common type spi. > 13. Use pcim_enable_device() and pcim_iomap_regions() in spi pci > driver. > 14. Passing the remapped address in loongson_spi_init_master. > 15. Remove the useless goto flag "err_out". > 16. Use pci vendor id in pci_ids.h. > 17. Use devm_platform_ioremap_resource in spi platform driver. > 18. Remove the useless item in pci_device_id. > 19. Remove the inned comma in of_device_id. > 20. Add some headfile in spi_loongson.h. > 21. Remove the useless extern for loongson_spi_init_master in > spi_loongson.h. > Change in v11: > 1. Use spi_get_chipselect() to replace all spi->chip_select in > spi driver > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/2] dt-bindings: spi: add loongson spi (no commit info) [2/2] spi: loongson: add bus driver for the loongson spi controller commit: 6c7a864007b66e60a3f64858a9555efed408b048 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2023-07-31 19:57 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-22 7:10 [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi Yinbo Zhu 2023-05-22 7:10 ` [PATCH v11 1/2] dt-bindings: spi: add " Yinbo Zhu 2023-05-24 8:56 ` Conor Dooley 2023-05-24 9:44 ` zhuyinbo 2023-05-24 10:29 ` Conor Dooley 2023-05-25 2:22 ` zhuyinbo [not found] ` <2196dd29-93ee-00f7-65b4-ede73aa8ba77@linaro.org> 2023-06-01 9:51 ` zhuyinbo 2023-06-01 15:30 ` Krzysztof Kozlowski 2023-06-02 6:46 ` zhuyinbo [not found] ` <69d355ff-90e1-09d2-d4ff-0d7dedc8addb@linaro.org> 2023-06-01 11:38 ` zhuyinbo 2023-05-22 7:10 ` [PATCH v11 2/2] spi: loongson: add bus driver for the loongson spi controller Yinbo Zhu 2023-05-23 12:54 ` andy.shevchenko 2023-05-24 7:52 ` zhuyinbo 2023-05-24 8:42 ` Andy Shevchenko 2023-05-24 10:19 ` Mark Brown 2023-05-25 3:34 ` zhuyinbo 2023-05-25 9:16 ` Andy Shevchenko 2023-05-25 9:28 ` zhuyinbo 2023-05-22 10:34 ` [PATCH v11 0/2] spi: loongson: add bus driver for the loongson spi Mark Brown 2023-05-22 11:44 ` zhuyinbo 2023-05-22 11:56 ` Mark Brown 2023-05-22 13:07 ` zhuyinbo 2023-05-22 13:10 ` Mark Brown 2023-05-23 2:08 ` zhuyinbo 2023-05-23 9:57 ` Mark Brown 2023-05-23 11:01 ` zhuyinbo 2023-07-31 19:57 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).