Re: [PATCH v2 2/4] spi: s3c64xx: support custom value of internal clock divider

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Chanho Park <chanho61.park@samsung.com>,
	Andi Shyti <andi@etezian.org>, Mark Brown <broonie@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>,
	devicetree@vger.kernel.org, linux-spi@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/4] spi: s3c64xx: support custom value of internal clock divider
Date: Wed, 29 Jun 2022 11:20:36 +0200	[thread overview]
Message-ID: <da86b25a-097d-63fe-083a-5600b72b0bdb@linaro.org> (raw)
In-Reply-To: <20220628044222.152794-3-chanho61.park@samsung.com>

On 28/06/2022 06:42, Chanho Park wrote:
> Modern exynos SoCs such as Exynos Auto v9 has different internal clock
> divider, for example "4". To support this internal value, this adds
> clk_div of the s3c64xx_spi_port_config and assign "2" as the default
> value to existing s3c64xx_spi_port_config.
> 
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index b3c50c7665fc..51a0e830441b 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -131,6 +131,7 @@ struct s3c64xx_spi_dma_data {
>   * @fifo_lvl_mask: Bit-mask for {TX|RX}_FIFO_LVL bits in SPI_STATUS register.
>   * @rx_lvl_offset: Bit offset of RX_FIFO_LVL bits in SPI_STATUS regiter.
>   * @tx_st_done: Bit offset of TX_DONE bit in SPI_STATUS regiter.
> + * @clk_div: Internal clock divider, if not specified, use 2 as the default.
>   * @quirks: Bitmask of known quirks
>   * @high_speed: True, if the controller supports HIGH_SPEED_EN bit.
>   * @clk_from_cmu: True, if the controller does not include a clock mux and
> @@ -148,6 +149,7 @@ struct s3c64xx_spi_port_config {
>  	int	rx_lvl_offset;
>  	int	tx_st_done;
>  	int	quirks;
> +	int	clk_div;
>  	bool	high_speed;
>  	bool	clk_from_cmu;
>  	bool	clk_ioclk;
> @@ -620,6 +622,7 @@ static int s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
>  	void __iomem *regs = sdd->regs;
>  	int ret;
>  	u32 val;
> +	u32 div = sdd->port_conf->clk_div;
>  
>  	/* Disable Clock */
>  	if (!sdd->port_conf->clk_from_cmu) {
> @@ -668,16 +671,15 @@ static int s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)
>  	writel(val, regs + S3C64XX_SPI_MODE_CFG);
>  
>  	if (sdd->port_conf->clk_from_cmu) {
> -		/* The src_clk clock is divided internally by 2 */
> -		ret = clk_set_rate(sdd->src_clk, sdd->cur_speed * 2);
> +		ret = clk_set_rate(sdd->src_clk, sdd->cur_speed * div);
>  		if (ret)
>  			return ret;
> -		sdd->cur_speed = clk_get_rate(sdd->src_clk) / 2;
> +		sdd->cur_speed = clk_get_rate(sdd->src_clk) / div;
>  	} else {
>  		/* Configure Clock */
>  		val = readl(regs + S3C64XX_SPI_CLK_CFG);
>  		val &= ~S3C64XX_SPI_PSR_MASK;
> -		val |= ((clk_get_rate(sdd->src_clk) / sdd->cur_speed / 2 - 1)
> +		val |= ((clk_get_rate(sdd->src_clk) / sdd->cur_speed / div - 1)
>  				& S3C64XX_SPI_PSR_MASK);
>  		writel(val, regs + S3C64XX_SPI_CLK_CFG);
>  
> @@ -871,6 +873,7 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>  	struct s3c64xx_spi_csinfo *cs = spi->controller_data;
>  	struct s3c64xx_spi_driver_data *sdd;
>  	int err;
> +	u32 div = 2;

This assignment is not effective - shortly later is being overwritten.

>  
>  	sdd = spi_master_get_devdata(spi->master);
>  	if (spi->dev.of_node) {
> @@ -889,22 +892,24 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>  
>  	pm_runtime_get_sync(&sdd->pdev->dev);
>  
> +	div = sdd->port_conf->clk_div;
> +

Best regards,
Krzysztof