diff mbox series

[v1,1/2] spi: spi-cadence: Add optional reset control support

Message ID 20240424051317.2084059-2-jisheng.teoh@starfivetech.com (mailing list archive)
State Superseded, archived
Headers show
Series Add optional reset control for Cadence SPI | expand

Commit Message

Ji Sheng Teoh April 24, 2024, 5:13 a.m. UTC 
Add optional reset control support for spi-cadence to properly bring
the SPI device into an operating condition.

Signed-off-by: Eng Lee Teh <englee.teh@starfivetech.com>
Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
Signed-off-by: Ji Sheng Teoh <jisheng.teoh@starfivetech.com>
---
 drivers/spi/spi-cadence.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

kernel test robot April 24, 2024, 4:26 p.m. UTC | #1 
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/202404250029.8cjM4mtZ-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240425/202404250029.8cjM4mtZ-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404250029.8cjM4mtZ-lkp@intel.com/

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
kernel test robot April 24, 2024, 6:40 p.m. UTC | #2 
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/202404250235.htdVhIMY-lkp@intel.com/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/202404250235.htdVhIMY-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404250235.htdVhIMY-lkp@intel.com/

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
Lars-Peter Clausen April 25, 2024, 1:54 a.m. UTC | #3 
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 <englee.teh@starfivetech.com>
> Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
> Signed-off-by: Ji Sheng Teoh <jisheng.teoh@starfivetech.com>
> ---
>   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)) {
diff mbox series

Patch

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)) {