diff mbox series

[3/5] spi: s3c64xx: support custom value of internal clock divider

Message ID 20220627064707.138883-4-chanho61.park@samsung.com (mailing list archive)
State New
Headers show
Series spi support for Exynos Auto v9 SoC | expand

Commit Message

Chanho Park June 27, 2022, 6:47 a.m. UTC
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 use it if it is specified.
Otherwise, use "2" which is the previous default value.

Signed-off-by: Chanho Park <chanho61.park@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

Comments

Krzysztof Kozlowski June 27, 2022, 9:40 a.m. UTC | #1
On 27/06/2022 08:47, 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 use it if it is specified.
> Otherwise, use "2" which is the previous default value.
> 
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index e17c74c0d7de..dd5fc8570bce 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 ? sdd->port_conf->clk_div : 2;

I would prefer to explicitly set '2' as clk_div for existing variants.
Such assignments in the code are usually trickier to find/read.
>  

Best regards,
Krzysztof
Chanho Park June 28, 2022, 1:51 a.m. UTC | #2
> Subject: Re: [PATCH 3/5] spi: s3c64xx: support custom value of internal
> clock divider
> 
> On 27/06/2022 08:47, 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 use it if it is specified.
> > Otherwise, use "2" which is the previous default value.
> >
> > Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> > ---
> >  drivers/spi/spi-s3c64xx.c | 23 +++++++++++++++--------
> >  1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> > index e17c74c0d7de..dd5fc8570bce 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 ? sdd->port_conf->clk_div : 2;
> 
> I would prefer to explicitly set '2' as clk_div for existing variants.
> Such assignments in the code are usually trickier to find/read.

Make sense. It can be more clear what value is using.
I'll apply it next patchset.

Best Regards,
Chanho Park
diff mbox series

Patch

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index e17c74c0d7de..dd5fc8570bce 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 ? sdd->port_conf->clk_div : 2;
 
 	/* 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;
 
 	sdd = spi_master_get_devdata(spi->master);
 	if (spi->dev.of_node) {
@@ -889,22 +892,26 @@  static int s3c64xx_spi_setup(struct spi_device *spi)
 
 	pm_runtime_get_sync(&sdd->pdev->dev);
 
+	/* Use clk_div value if it is specified, otherwise use 2 as default */
+	if (sdd->port_conf->clk_div)
+		div = sdd->port_conf->clk_div;
+
 	/* Check if we can provide the requested rate */
 	if (!sdd->port_conf->clk_from_cmu) {
 		u32 psr, speed;
 
 		/* Max possible */
-		speed = clk_get_rate(sdd->src_clk) / 2 / (0 + 1);
+		speed = clk_get_rate(sdd->src_clk) / div / (0 + 1);
 
 		if (spi->max_speed_hz > speed)
 			spi->max_speed_hz = speed;
 
-		psr = clk_get_rate(sdd->src_clk) / 2 / spi->max_speed_hz - 1;
+		psr = clk_get_rate(sdd->src_clk) / div / spi->max_speed_hz - 1;
 		psr &= S3C64XX_SPI_PSR_MASK;
 		if (psr == S3C64XX_SPI_PSR_MASK)
 			psr--;
 
-		speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
+		speed = clk_get_rate(sdd->src_clk) / div / (psr + 1);
 		if (spi->max_speed_hz < speed) {
 			if (psr+1 < S3C64XX_SPI_PSR_MASK) {
 				psr++;
@@ -914,7 +921,7 @@  static int s3c64xx_spi_setup(struct spi_device *spi)
 			}
 		}
 
-		speed = clk_get_rate(sdd->src_clk) / 2 / (psr + 1);
+		speed = clk_get_rate(sdd->src_clk) / div / (psr + 1);
 		if (spi->max_speed_hz >= speed) {
 			spi->max_speed_hz = speed;
 		} else {