2024-04-24 05:14:14

by JiSheng Teoh

[permalink] [raw]
Subject: [PATCH v1 1/2] spi: spi-cadence: Add optional reset control support

Add optional reset control support for spi-cadence to properly bring
the SPI device into an operating condition.

Signed-off-by: Eng Lee Teh <[email protected]>
Signed-off-by: Ley Foon Tan <[email protected]>
Signed-off-by: Ji Sheng Teoh <[email protected]>
---
drivers/spi/spi-cadence.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
index e5140532071d..41f2f51d39e4 100644
--- a/drivers/spi/spi-cadence.c
+++ b/drivers/spi/spi-cadence.c
@@ -111,6 +111,7 @@
* @dev_busy: Device busy flag
* @is_decoded_cs: Flag for decoder property set or not
* @tx_fifo_depth: Depth of the TX FIFO
+ * @rstc: Optional reset control for SPI controller
*/
struct cdns_spi {
void __iomem *regs;
@@ -125,6 +126,7 @@ struct cdns_spi {
u8 dev_busy;
u32 is_decoded_cs;
unsigned int tx_fifo_depth;
+ struct reset_control *rstc;
};

/* Macros for the SPI controller read/write */
@@ -588,6 +590,16 @@ static int cdns_spi_probe(struct platform_device *pdev)
goto remove_ctlr;
}

+ xspi->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(xspi->rstc)) {
+ ret = PTR_ERR(xspi->rstc);
+ dev_err(&pdev->dev, "Cannot get SPI reset.\n");
+ goto remove_ctlr;
+ }
+
+ reset_control_assert(xspi->rstc);
+ reset_control_deassert(xspi->rstc);
+
if (!spi_controller_is_target(ctlr)) {
xspi->ref_clk = devm_clk_get_enabled(&pdev->dev, "ref_clk");
if (IS_ERR(xspi->ref_clk)) {
--
2.43.2



2024-04-24 16:26:36

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] spi: spi-cadence: Add optional reset control support

Hi Ji,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-spi/for-next]
[also build test ERROR on robh/for-next krzk-dt/for-next linus/master v6.9-rc5 next-20240424]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Ji-Sheng-Teoh/spi-spi-cadence-Add-optional-reset-control-support/20240424-131551
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link: https://lore.kernel.org/r/20240424051317.2084059-2-jisheng.teoh%40starfivetech.com
patch subject: [PATCH v1 1/2] spi: spi-cadence: Add optional reset control support
config: nios2-randconfig-r081-20240424 (https://download.01.org/0day-ci/archive/20240425/[email protected]/config)
compiler: nios2-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240425/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All error/warnings (new ones prefixed by >>):

drivers/spi/spi-cadence.c: In function 'cdns_spi_probe':
>> drivers/spi/spi-cadence.c:593:22: error: implicit declaration of function 'devm_reset_control_get_optional_exclusive' [-Werror=implicit-function-declaration]
593 | xspi->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/spi/spi-cadence.c:593:20: warning: assignment to 'struct reset_control *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
593 | xspi->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
| ^
>> drivers/spi/spi-cadence.c:600:9: error: implicit declaration of function 'reset_control_assert' [-Werror=implicit-function-declaration]
600 | reset_control_assert(xspi->rstc);
| ^~~~~~~~~~~~~~~~~~~~
>> drivers/spi/spi-cadence.c:601:9: error: implicit declaration of function 'reset_control_deassert' [-Werror=implicit-function-declaration]
601 | reset_control_deassert(xspi->rstc);
| ^~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors


vim +/devm_reset_control_get_optional_exclusive +593 drivers/spi/spi-cadence.c

550
551 /**
552 * cdns_spi_probe - Probe method for the SPI driver
553 * @pdev: Pointer to the platform_device structure
554 *
555 * This function initializes the driver data structures and the hardware.
556 *
557 * Return: 0 on success and error value on error
558 */
559 static int cdns_spi_probe(struct platform_device *pdev)
560 {
561 int ret = 0, irq;
562 struct spi_controller *ctlr;
563 struct cdns_spi *xspi;
564 u32 num_cs;
565 bool target;
566
567 target = of_property_read_bool(pdev->dev.of_node, "spi-slave");
568 if (target)
569 ctlr = spi_alloc_target(&pdev->dev, sizeof(*xspi));
570 else
571 ctlr = spi_alloc_host(&pdev->dev, sizeof(*xspi));
572
573 if (!ctlr)
574 return -ENOMEM;
575
576 xspi = spi_controller_get_devdata(ctlr);
577 ctlr->dev.of_node = pdev->dev.of_node;
578 platform_set_drvdata(pdev, ctlr);
579
580 xspi->regs = devm_platform_ioremap_resource(pdev, 0);
581 if (IS_ERR(xspi->regs)) {
582 ret = PTR_ERR(xspi->regs);
583 goto remove_ctlr;
584 }
585
586 xspi->pclk = devm_clk_get_enabled(&pdev->dev, "pclk");
587 if (IS_ERR(xspi->pclk)) {
588 dev_err(&pdev->dev, "pclk clock not found.\n");
589 ret = PTR_ERR(xspi->pclk);
590 goto remove_ctlr;
591 }
592
> 593 xspi->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
594 if (IS_ERR(xspi->rstc)) {
595 ret = PTR_ERR(xspi->rstc);
596 dev_err(&pdev->dev, "Cannot get SPI reset.\n");
597 goto remove_ctlr;
598 }
599
> 600 reset_control_assert(xspi->rstc);
> 601 reset_control_deassert(xspi->rstc);
602
603 if (!spi_controller_is_target(ctlr)) {
604 xspi->ref_clk = devm_clk_get_enabled(&pdev->dev, "ref_clk");
605 if (IS_ERR(xspi->ref_clk)) {
606 dev_err(&pdev->dev, "ref_clk clock not found.\n");
607 ret = PTR_ERR(xspi->ref_clk);
608 goto remove_ctlr;
609 }
610
611 pm_runtime_use_autosuspend(&pdev->dev);
612 pm_runtime_set_autosuspend_delay(&pdev->dev, SPI_AUTOSUSPEND_TIMEOUT);
613 pm_runtime_get_noresume(&pdev->dev);
614 pm_runtime_set_active(&pdev->dev);
615 pm_runtime_enable(&pdev->dev);
616
617 ret = of_property_read_u32(pdev->dev.of_node, "num-cs", &num_cs);
618 if (ret < 0)
619 ctlr->num_chipselect = CDNS_SPI_DEFAULT_NUM_CS;
620 else
621 ctlr->num_chipselect = num_cs;
622
623 ret = of_property_read_u32(pdev->dev.of_node, "is-decoded-cs",
624 &xspi->is_decoded_cs);
625 if (ret < 0)
626 xspi->is_decoded_cs = 0;
627 }
628
629 cdns_spi_detect_fifo_depth(xspi);
630
631 /* SPI controller initializations */
632 cdns_spi_init_hw(xspi, spi_controller_is_target(ctlr));
633
634 irq = platform_get_irq(pdev, 0);
635 if (irq < 0) {
636 ret = irq;
637 goto clk_dis_all;
638 }
639
640 ret = devm_request_irq(&pdev->dev, irq, cdns_spi_irq,
641 0, pdev->name, ctlr);
642 if (ret != 0) {
643 ret = -ENXIO;
644 dev_err(&pdev->dev, "request_irq failed\n");
645 goto clk_dis_all;
646 }
647
648 ctlr->use_gpio_descriptors = true;
649 ctlr->prepare_transfer_hardware = cdns_prepare_transfer_hardware;
650 ctlr->prepare_message = cdns_prepare_message;
651 ctlr->transfer_one = cdns_transfer_one;
652 ctlr->unprepare_transfer_hardware = cdns_unprepare_transfer_hardware;
653 ctlr->mode_bits = SPI_CPOL | SPI_CPHA;
654 ctlr->bits_per_word_mask = SPI_BPW_MASK(8);
655
656 if (!spi_controller_is_target(ctlr)) {
657 ctlr->mode_bits |= SPI_CS_HIGH;
658 ctlr->set_cs = cdns_spi_chipselect;
659 ctlr->auto_runtime_pm = true;
660 xspi->clk_rate = clk_get_rate(xspi->ref_clk);
661 /* Set to default valid value */
662 ctlr->max_speed_hz = xspi->clk_rate / 4;
663 xspi->speed_hz = ctlr->max_speed_hz;
664 pm_runtime_mark_last_busy(&pdev->dev);
665 pm_runtime_put_autosuspend(&pdev->dev);
666 } else {
667 ctlr->mode_bits |= SPI_NO_CS;
668 ctlr->target_abort = cdns_target_abort;
669 }
670 ret = spi_register_controller(ctlr);
671 if (ret) {
672 dev_err(&pdev->dev, "spi_register_controller failed\n");
673 goto clk_dis_all;
674 }
675
676 return ret;
677
678 clk_dis_all:
679 if (!spi_controller_is_target(ctlr)) {
680 pm_runtime_set_suspended(&pdev->dev);
681 pm_runtime_disable(&pdev->dev);
682 }
683 remove_ctlr:
684 spi_controller_put(ctlr);
685 return ret;
686 }
687

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-04-24 19:04:27

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] spi: spi-cadence: Add optional reset control support

Hi Ji,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-spi/for-next]
[also build test ERROR on robh/for-next krzk-dt/for-next linus/master v6.9-rc5 next-20240424]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Ji-Sheng-Teoh/spi-spi-cadence-Add-optional-reset-control-support/20240424-131551
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link: https://lore.kernel.org/r/20240424051317.2084059-2-jisheng.teoh%40starfivetech.com
patch subject: [PATCH v1 1/2] spi: spi-cadence: Add optional reset control support
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240425/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240425/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All error/warnings (new ones prefixed by >>):

>> drivers/spi/spi-cadence.c:593:15: error: implicit declaration of function 'devm_reset_control_get_optional_exclusive' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
xspi->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
^
>> drivers/spi/spi-cadence.c:593:13: warning: incompatible integer to pointer conversion assigning to 'struct reset_control *' from 'int' [-Wint-conversion]
xspi->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/spi/spi-cadence.c:600:2: error: implicit declaration of function 'reset_control_assert' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
reset_control_assert(xspi->rstc);
^
>> drivers/spi/spi-cadence.c:601:2: error: implicit declaration of function 'reset_control_deassert' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
reset_control_deassert(xspi->rstc);
^
drivers/spi/spi-cadence.c:601:2: note: did you mean 'reset_control_assert'?
drivers/spi/spi-cadence.c:600:2: note: 'reset_control_assert' declared here
reset_control_assert(xspi->rstc);
^
1 warning and 3 errors generated.


vim +/devm_reset_control_get_optional_exclusive +593 drivers/spi/spi-cadence.c

550
551 /**
552 * cdns_spi_probe - Probe method for the SPI driver
553 * @pdev: Pointer to the platform_device structure
554 *
555 * This function initializes the driver data structures and the hardware.
556 *
557 * Return: 0 on success and error value on error
558 */
559 static int cdns_spi_probe(struct platform_device *pdev)
560 {
561 int ret = 0, irq;
562 struct spi_controller *ctlr;
563 struct cdns_spi *xspi;
564 u32 num_cs;
565 bool target;
566
567 target = of_property_read_bool(pdev->dev.of_node, "spi-slave");
568 if (target)
569 ctlr = spi_alloc_target(&pdev->dev, sizeof(*xspi));
570 else
571 ctlr = spi_alloc_host(&pdev->dev, sizeof(*xspi));
572
573 if (!ctlr)
574 return -ENOMEM;
575
576 xspi = spi_controller_get_devdata(ctlr);
577 ctlr->dev.of_node = pdev->dev.of_node;
578 platform_set_drvdata(pdev, ctlr);
579
580 xspi->regs = devm_platform_ioremap_resource(pdev, 0);
581 if (IS_ERR(xspi->regs)) {
582 ret = PTR_ERR(xspi->regs);
583 goto remove_ctlr;
584 }
585
586 xspi->pclk = devm_clk_get_enabled(&pdev->dev, "pclk");
587 if (IS_ERR(xspi->pclk)) {
588 dev_err(&pdev->dev, "pclk clock not found.\n");
589 ret = PTR_ERR(xspi->pclk);
590 goto remove_ctlr;
591 }
592
> 593 xspi->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
594 if (IS_ERR(xspi->rstc)) {
595 ret = PTR_ERR(xspi->rstc);
596 dev_err(&pdev->dev, "Cannot get SPI reset.\n");
597 goto remove_ctlr;
598 }
599
> 600 reset_control_assert(xspi->rstc);
> 601 reset_control_deassert(xspi->rstc);
602
603 if (!spi_controller_is_target(ctlr)) {
604 xspi->ref_clk = devm_clk_get_enabled(&pdev->dev, "ref_clk");
605 if (IS_ERR(xspi->ref_clk)) {
606 dev_err(&pdev->dev, "ref_clk clock not found.\n");
607 ret = PTR_ERR(xspi->ref_clk);
608 goto remove_ctlr;
609 }
610
611 pm_runtime_use_autosuspend(&pdev->dev);
612 pm_runtime_set_autosuspend_delay(&pdev->dev, SPI_AUTOSUSPEND_TIMEOUT);
613 pm_runtime_get_noresume(&pdev->dev);
614 pm_runtime_set_active(&pdev->dev);
615 pm_runtime_enable(&pdev->dev);
616
617 ret = of_property_read_u32(pdev->dev.of_node, "num-cs", &num_cs);
618 if (ret < 0)
619 ctlr->num_chipselect = CDNS_SPI_DEFAULT_NUM_CS;
620 else
621 ctlr->num_chipselect = num_cs;
622
623 ret = of_property_read_u32(pdev->dev.of_node, "is-decoded-cs",
624 &xspi->is_decoded_cs);
625 if (ret < 0)
626 xspi->is_decoded_cs = 0;
627 }
628
629 cdns_spi_detect_fifo_depth(xspi);
630
631 /* SPI controller initializations */
632 cdns_spi_init_hw(xspi, spi_controller_is_target(ctlr));
633
634 irq = platform_get_irq(pdev, 0);
635 if (irq < 0) {
636 ret = irq;
637 goto clk_dis_all;
638 }
639
640 ret = devm_request_irq(&pdev->dev, irq, cdns_spi_irq,
641 0, pdev->name, ctlr);
642 if (ret != 0) {
643 ret = -ENXIO;
644 dev_err(&pdev->dev, "request_irq failed\n");
645 goto clk_dis_all;
646 }
647
648 ctlr->use_gpio_descriptors = true;
649 ctlr->prepare_transfer_hardware = cdns_prepare_transfer_hardware;
650 ctlr->prepare_message = cdns_prepare_message;
651 ctlr->transfer_one = cdns_transfer_one;
652 ctlr->unprepare_transfer_hardware = cdns_unprepare_transfer_hardware;
653 ctlr->mode_bits = SPI_CPOL | SPI_CPHA;
654 ctlr->bits_per_word_mask = SPI_BPW_MASK(8);
655
656 if (!spi_controller_is_target(ctlr)) {
657 ctlr->mode_bits |= SPI_CS_HIGH;
658 ctlr->set_cs = cdns_spi_chipselect;
659 ctlr->auto_runtime_pm = true;
660 xspi->clk_rate = clk_get_rate(xspi->ref_clk);
661 /* Set to default valid value */
662 ctlr->max_speed_hz = xspi->clk_rate / 4;
663 xspi->speed_hz = ctlr->max_speed_hz;
664 pm_runtime_mark_last_busy(&pdev->dev);
665 pm_runtime_put_autosuspend(&pdev->dev);
666 } else {
667 ctlr->mode_bits |= SPI_NO_CS;
668 ctlr->target_abort = cdns_target_abort;
669 }
670 ret = spi_register_controller(ctlr);
671 if (ret) {
672 dev_err(&pdev->dev, "spi_register_controller failed\n");
673 goto clk_dis_all;
674 }
675
676 return ret;
677
678 clk_dis_all:
679 if (!spi_controller_is_target(ctlr)) {
680 pm_runtime_set_suspended(&pdev->dev);
681 pm_runtime_disable(&pdev->dev);
682 }
683 remove_ctlr:
684 spi_controller_put(ctlr);
685 return ret;
686 }
687

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-04-25 02:11:56

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] spi: spi-cadence: Add optional reset control support

On 4/23/24 22:13, Ji Sheng Teoh wrote:
> Add optional reset control support for spi-cadence to properly bring
> the SPI device into an operating condition.
>
> Signed-off-by: Eng Lee Teh <[email protected]>
> Signed-off-by: Ley Foon Tan <[email protected]>
> Signed-off-by: Ji Sheng Teoh <[email protected]>
> ---
> drivers/spi/spi-cadence.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
> index e5140532071d..41f2f51d39e4 100644
> --- a/drivers/spi/spi-cadence.c
> +++ b/drivers/spi/spi-cadence.c
> @@ -111,6 +111,7 @@
> * @dev_busy: Device busy flag
> * @is_decoded_cs: Flag for decoder property set or not
> * @tx_fifo_depth: Depth of the TX FIFO
> + * @rstc: Optional reset control for SPI controller
> */
> struct cdns_spi {
> void __iomem *regs;
> @@ -125,6 +126,7 @@ struct cdns_spi {
> u8 dev_busy;
> u32 is_decoded_cs;
> unsigned int tx_fifo_depth;
> + struct reset_control *rstc;
> };
>
> /* Macros for the SPI controller read/write */
> @@ -588,6 +590,16 @@ static int cdns_spi_probe(struct platform_device *pdev)
> goto remove_ctlr;
> }
>
> + xspi->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);

The cadence SPI core has 3 different resets signals. Maybe use a name
for the reset to make it clear which reset this is referring to.

> + if (IS_ERR(xspi->rstc)) {
> + ret = PTR_ERR(xspi->rstc);
> + dev_err(&pdev->dev, "Cannot get SPI reset.\n");
> + goto remove_ctlr;
> + }
> +
> + reset_control_assert(xspi->rstc);
> + reset_control_deassert(xspi->rstc);
> +
> if (!spi_controller_is_target(ctlr)) {
> xspi->ref_clk = devm_clk_get_enabled(&pdev->dev, "ref_clk");
> if (IS_ERR(xspi->ref_clk)) {