| Message ID | BYAPR20MB24728A99C2F7505873A091B5BCF79@BYAPR20MB2472.namprd20.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] dt-bindings: spi: sun6i: add DT bindings for Allwinner R329 SPI | expand |
On 4/22/22 10:56 AM, icenowy@outlook.com wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > Previously SPI controllers in Allwinner SoCs has a clock divider inside. > However now the clock divider is removed and to set the transfer clock > rate it's only needed to set the SPI module clock to the target value. > > Add a quirk for this kind of SPI controllers. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > --- > drivers/spi/spi-sun6i.c | 68 +++++++++++++++++++++++------------------ > 1 file changed, 38 insertions(+), 30 deletions(-) > > diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c > index 84c525b08ad0..fc81afc3a963 100644 > --- a/drivers/spi/spi-sun6i.c > +++ b/drivers/spi/spi-sun6i.c > @@ -87,6 +87,7 @@ > > struct sun6i_spi_cfg { > unsigned long fifo_depth; > + bool has_clk_ctl; > }; > > struct sun6i_spi { > @@ -260,7 +261,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master, > struct spi_transfer *tfr) > { > struct sun6i_spi *sspi = spi_master_get_devdata(master); > - unsigned int mclk_rate, div, div_cdr1, div_cdr2, timeout; > + unsigned int div, div_cdr1, div_cdr2, timeout; > unsigned int start, end, tx_time; > unsigned int trig_level; > unsigned int tx_len = 0, rx_len = 0; > @@ -350,39 +351,44 @@ static int sun6i_spi_transfer_one(struct spi_master *master, > > sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg); > > - /* Ensure that we have a parent clock fast enough */ > - mclk_rate = clk_get_rate(sspi->mclk); > - if (mclk_rate < (2 * tfr->speed_hz)) { > - clk_set_rate(sspi->mclk, 2 * tfr->speed_hz); > - mclk_rate = clk_get_rate(sspi->mclk); > - } > + if (sspi->cfg->has_clk_ctl) { > + unsigned int mclk_rate = clk_get_rate(sspi->mclk); > + /* Ensure that we have a parent clock fast enough */ > + if (mclk_rate < (2 * tfr->speed_hz)) { > + clk_set_rate(sspi->mclk, 2 * tfr->speed_hz); > + mclk_rate = clk_get_rate(sspi->mclk); > + } > > - /* > - * Setup clock divider. > - * > - * We have two choices there. Either we can use the clock > - * divide rate 1, which is calculated thanks to this formula: > - * SPI_CLK = MOD_CLK / (2 ^ cdr) > - * Or we can use CDR2, which is calculated with the formula: > - * SPI_CLK = MOD_CLK / (2 * (cdr + 1)) > - * Wether we use the former or the latter is set through the > - * DRS bit. > - * > - * First try CDR2, and if we can't reach the expected > - * frequency, fall back to CDR1. > - */ > - div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz); > - div_cdr2 = DIV_ROUND_UP(div_cdr1, 2); > - if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) { > - reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN6I_CLK_CTL_DRS; > - tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2); > + /* > + * Setup clock divider. > + * > + * We have two choices there. Either we can use the clock > + * divide rate 1, which is calculated thanks to this formula: > + * SPI_CLK = MOD_CLK / (2 ^ cdr) > + * Or we can use CDR2, which is calculated with the formula: > + * SPI_CLK = MOD_CLK / (2 * (cdr + 1)) > + * Wether we use the former or the latter is set through the > + * DRS bit. > + * > + * First try CDR2, and if we can't reach the expected > + * frequency, fall back to CDR1. > + */ > + div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz); > + div_cdr2 = DIV_ROUND_UP(div_cdr1, 2); > + if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) { > + reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN6I_CLK_CTL_DRS; > + tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2); > + } else { > + div = min(SUN6I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1)); > + reg = SUN6I_CLK_CTL_CDR1(div); > + tfr->effective_speed_hz = mclk_rate / (1 << div); > + } > + > + sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg); > } else { > - div = min(SUN6I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1)); > - reg = SUN6I_CLK_CTL_CDR1(div); > - tfr->effective_speed_hz = mclk_rate / (1 << div); > + clk_set_rate(sspi->mclk, tfr->speed_hz); clk_set_rate() may not set the exact requested rate. Should we set tfr->effective_speed_hz based on clk_get_rate() afterward? Regards, Samuel > } > > - sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg); > /* Finally enable the bus - doing so before might raise SCK to HIGH */ > reg = sun6i_spi_read(sspi, SUN6I_GBL_CTL_REG); > reg |= SUN6I_GBL_CTL_BUS_ENABLE; > @@ -702,10 +708,12 @@ static int sun6i_spi_remove(struct platform_device *pdev) > > static const struct sun6i_spi_cfg sun6i_a31_spi_cfg = { > .fifo_depth = SUN6I_FIFO_DEPTH, > + .has_clk_ctl = true, > }; > > static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = { > .fifo_depth = SUN8I_FIFO_DEPTH, > + .has_clk_ctl = true, > }; > > static const struct of_device_id sun6i_spi_match[] = { >
在 2022-04-22星期五的 18:54 -0500，Samuel Holland写道： > On 4/22/22 10:56 AM, icenowy@outlook.com wrote: > > From: Icenowy Zheng <icenowy@aosc.io> > > > > Previously SPI controllers in Allwinner SoCs has a clock divider > > inside. > > However now the clock divider is removed and to set the transfer > > clock > > rate it's only needed to set the SPI module clock to the target > > value. > > > > Add a quirk for this kind of SPI controllers. > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > --- > > drivers/spi/spi-sun6i.c | 68 +++++++++++++++++++++++-------------- > > ---- > > 1 file changed, 38 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c > > index 84c525b08ad0..fc81afc3a963 100644 > > --- a/drivers/spi/spi-sun6i.c > > +++ b/drivers/spi/spi-sun6i.c > > @@ -87,6 +87,7 @@ > > > > struct sun6i_spi_cfg { > > unsigned long fifo_depth; > > + bool has_clk_ctl; > > }; > > > > struct sun6i_spi { > > @@ -260,7 +261,7 @@ static int sun6i_spi_transfer_one(struct > > spi_master *master, > > struct spi_transfer *tfr) > > { > > struct sun6i_spi *sspi = spi_master_get_devdata(master); > > - unsigned int mclk_rate, div, div_cdr1, div_cdr2, timeout; > > + unsigned int div, div_cdr1, div_cdr2, timeout; > > unsigned int start, end, tx_time; > > unsigned int trig_level; > > unsigned int tx_len = 0, rx_len = 0; > > @@ -350,39 +351,44 @@ static int sun6i_spi_transfer_one(struct > > spi_master *master, > > > > sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg); > > > > - /* Ensure that we have a parent clock fast enough */ > > - mclk_rate = clk_get_rate(sspi->mclk); > > - if (mclk_rate < (2 * tfr->speed_hz)) { > > - clk_set_rate(sspi->mclk, 2 * tfr->speed_hz); > > - mclk_rate = clk_get_rate(sspi->mclk); > > - } > > + if (sspi->cfg->has_clk_ctl) { > > + unsigned int mclk_rate = clk_get_rate(sspi->mclk); > > + /* Ensure that we have a parent clock fast enough > > */ > > + if (mclk_rate < (2 * tfr->speed_hz)) { > > + clk_set_rate(sspi->mclk, 2 * tfr- > > >speed_hz); > > + mclk_rate = clk_get_rate(sspi->mclk); > > + } > > > > - /* > > - * Setup clock divider. > > - * > > - * We have two choices there. Either we can use the clock > > - * divide rate 1, which is calculated thanks to this > > formula: > > - * SPI_CLK = MOD_CLK / (2 ^ cdr) > > - * Or we can use CDR2, which is calculated with the > > formula: > > - * SPI_CLK = MOD_CLK / (2 * (cdr + 1)) > > - * Wether we use the former or the latter is set through > > the > > - * DRS bit. > > - * > > - * First try CDR2, and if we can't reach the expected > > - * frequency, fall back to CDR1. > > - */ > > - div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz); > > - div_cdr2 = DIV_ROUND_UP(div_cdr1, 2); > > - if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) { > > - reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | > > SUN6I_CLK_CTL_DRS; > > - tfr->effective_speed_hz = mclk_rate / (2 * > > div_cdr2); > > + /* > > + * Setup clock divider. > > + * > > + * We have two choices there. Either we can use the > > clock > > + * divide rate 1, which is calculated thanks to > > this formula: > > + * SPI_CLK = MOD_CLK / (2 ^ cdr) > > + * Or we can use CDR2, which is calculated with the > > formula: > > + * SPI_CLK = MOD_CLK / (2 * (cdr + 1)) > > + * Wether we use the former or the latter is set > > through the > > + * DRS bit. > > + * > > + * First try CDR2, and if we can't reach the > > expected > > + * frequency, fall back to CDR1. > > + */ > > + div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz); > > + div_cdr2 = DIV_ROUND_UP(div_cdr1, 2); > > + if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) { > > + reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | > > SUN6I_CLK_CTL_DRS; > > + tfr->effective_speed_hz = mclk_rate / (2 * > > div_cdr2); > > + } else { > > + div = min(SUN6I_CLK_CTL_CDR1_MASK, > > order_base_2(div_cdr1)); > > + reg = SUN6I_CLK_CTL_CDR1(div); > > + tfr->effective_speed_hz = mclk_rate / (1 << > > div); > > + } > > + > > + sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg); > > } else { > > - div = min(SUN6I_CLK_CTL_CDR1_MASK, > > order_base_2(div_cdr1)); > > - reg = SUN6I_CLK_CTL_CDR1(div); > > - tfr->effective_speed_hz = mclk_rate / (1 << div); > > + clk_set_rate(sspi->mclk, tfr->speed_hz); > > clk_set_rate() may not set the exact requested rate. Should we set > tfr->effective_speed_hz based on clk_get_rate() afterward? Sounds right. I should add this in the next revision. > > Regards, > Samuel > > > } > > > > - sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg); > > /* Finally enable the bus - doing so before might raise SCK > > to HIGH */ > > reg = sun6i_spi_read(sspi, SUN6I_GBL_CTL_REG); > > reg |= SUN6I_GBL_CTL_BUS_ENABLE; > > @@ -702,10 +708,12 @@ static int sun6i_spi_remove(struct > > platform_device *pdev) > > > > static const struct sun6i_spi_cfg sun6i_a31_spi_cfg = { > > .fifo_depth = SUN6I_FIFO_DEPTH, > > + .has_clk_ctl = true, > > }; > > > > static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = { > > .fifo_depth = SUN8I_FIFO_DEPTH, > > + .has_clk_ctl = true, > > }; > > > > static const struct of_device_id sun6i_spi_match[] = { > > >
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c index 84c525b08ad0..fc81afc3a963 100644 --- a/drivers/spi/spi-sun6i.c +++ b/drivers/spi/spi-sun6i.c @@ -87,6 +87,7 @@ struct sun6i_spi_cfg { unsigned long fifo_depth; + bool has_clk_ctl; }; struct sun6i_spi { @@ -260,7 +261,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master, struct spi_transfer *tfr) { struct sun6i_spi *sspi = spi_master_get_devdata(master); - unsigned int mclk_rate, div, div_cdr1, div_cdr2, timeout; + unsigned int div, div_cdr1, div_cdr2, timeout; unsigned int start, end, tx_time; unsigned int trig_level; unsigned int tx_len = 0, rx_len = 0; @@ -350,39 +351,44 @@ static int sun6i_spi_transfer_one(struct spi_master *master, sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg); - /* Ensure that we have a parent clock fast enough */ - mclk_rate = clk_get_rate(sspi->mclk); - if (mclk_rate < (2 * tfr->speed_hz)) { - clk_set_rate(sspi->mclk, 2 * tfr->speed_hz); - mclk_rate = clk_get_rate(sspi->mclk); - } + if (sspi->cfg->has_clk_ctl) { + unsigned int mclk_rate = clk_get_rate(sspi->mclk); + /* Ensure that we have a parent clock fast enough */ + if (mclk_rate < (2 * tfr->speed_hz)) { + clk_set_rate(sspi->mclk, 2 * tfr->speed_hz); + mclk_rate = clk_get_rate(sspi->mclk); + } - /* - * Setup clock divider. - * - * We have two choices there. Either we can use the clock - * divide rate 1, which is calculated thanks to this formula: - * SPI_CLK = MOD_CLK / (2 ^ cdr) - * Or we can use CDR2, which is calculated with the formula: - * SPI_CLK = MOD_CLK / (2 * (cdr + 1)) - * Wether we use the former or the latter is set through the - * DRS bit. - * - * First try CDR2, and if we can't reach the expected - * frequency, fall back to CDR1. - */ - div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz); - div_cdr2 = DIV_ROUND_UP(div_cdr1, 2); - if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) { - reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN6I_CLK_CTL_DRS; - tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2); + /* + * Setup clock divider. + * + * We have two choices there. Either we can use the clock + * divide rate 1, which is calculated thanks to this formula: + * SPI_CLK = MOD_CLK / (2 ^ cdr) + * Or we can use CDR2, which is calculated with the formula: + * SPI_CLK = MOD_CLK / (2 * (cdr + 1)) + * Wether we use the former or the latter is set through the + * DRS bit. + * + * First try CDR2, and if we can't reach the expected + * frequency, fall back to CDR1. + */ + div_cdr1 = DIV_ROUND_UP(mclk_rate, tfr->speed_hz); + div_cdr2 = DIV_ROUND_UP(div_cdr1, 2); + if (div_cdr2 <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) { + reg = SUN6I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN6I_CLK_CTL_DRS; + tfr->effective_speed_hz = mclk_rate / (2 * div_cdr2); + } else { + div = min(SUN6I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1)); + reg = SUN6I_CLK_CTL_CDR1(div); + tfr->effective_speed_hz = mclk_rate / (1 << div); + } + + sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg); } else { - div = min(SUN6I_CLK_CTL_CDR1_MASK, order_base_2(div_cdr1)); - reg = SUN6I_CLK_CTL_CDR1(div); - tfr->effective_speed_hz = mclk_rate / (1 << div); + clk_set_rate(sspi->mclk, tfr->speed_hz); } - sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg); /* Finally enable the bus - doing so before might raise SCK to HIGH */ reg = sun6i_spi_read(sspi, SUN6I_GBL_CTL_REG); reg |= SUN6I_GBL_CTL_BUS_ENABLE; @@ -702,10 +708,12 @@ static int sun6i_spi_remove(struct platform_device *pdev) static const struct sun6i_spi_cfg sun6i_a31_spi_cfg = { .fifo_depth = SUN6I_FIFO_DEPTH, + .has_clk_ctl = true, }; static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = { .fifo_depth = SUN8I_FIFO_DEPTH, + .has_clk_ctl = true, }; static const struct of_device_id sun6i_spi_match[] = {