Hi Iwamatsu-san,
On Wed, Jan 14, 2015 at 8:25 AM, Nobuhiro Iwamatsu
<[email protected]> wrote:
> sh-msiof of frequency dividing does not perform the calculation, driver have
> to manage setting value in the table. It is not possible to set frequency
> dividing value close to the actual data in this way. This changes from
> frequency dividing of table management to setting by calculation.
> This driver is able to set a value close to the actual data.
Thanks for your patch!
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 96a5fc0..58b1bfe 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -241,42 +241,37 @@ static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
>
> static struct {
> unsigned short div;
> - unsigned short scr;
> -} const sh_msiof_spi_clk_table[] = {
> - { 1, SCR_BRPS( 1) | SCR_BRDV_DIV_1 },
> - { 2, SCR_BRPS( 1) | SCR_BRDV_DIV_2 },
> - { 4, SCR_BRPS( 1) | SCR_BRDV_DIV_4 },
> - { 8, SCR_BRPS( 1) | SCR_BRDV_DIV_8 },
> - { 16, SCR_BRPS( 1) | SCR_BRDV_DIV_16 },
> - { 32, SCR_BRPS( 1) | SCR_BRDV_DIV_32 },
> - { 64, SCR_BRPS(32) | SCR_BRDV_DIV_2 },
> - { 128, SCR_BRPS(32) | SCR_BRDV_DIV_4 },
> - { 256, SCR_BRPS(32) | SCR_BRDV_DIV_8 },
> - { 512, SCR_BRPS(32) | SCR_BRDV_DIV_16 },
> - { 1024, SCR_BRPS(32) | SCR_BRDV_DIV_32 },
> + unsigned short brdv;
> +} const sh_msiof_spi_div_table[] = {
> + { 1, SCR_BRDV_DIV_1 },
> + { 2, SCR_BRDV_DIV_2 },
> + { 4, SCR_BRDV_DIV_4 },
> + { 8, SCR_BRDV_DIV_8 },
> + { 16, SCR_BRDV_DIV_16 },
> + { 32, SCR_BRDV_DIV_32 },
> };
>
> static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p,
> unsigned long parent_rate, u32 spi_hz)
> {
> unsigned long div = 1024;
> + unsigned long brps, scr;
"u32", as these are (parts of) 32-bit register values.
> size_t k;
>
> if (!WARN_ON(!spi_hz || !parent_rate))
> div = DIV_ROUND_UP(parent_rate, spi_hz);
>
> - /* TODO: make more fine grained */
> -
> - for (k = 0; k < ARRAY_SIZE(sh_msiof_spi_clk_table); k++) {
> - if (sh_msiof_spi_clk_table[k].div >= div)
> - break;
> + for (k = 0; k < ARRAY_SIZE(sh_msiof_spi_div_table); k++) {
> + brps = DIV_ROUND_UP(div, sh_msiof_spi_div_table[k].div);
> + if (brps > 32) /* max of brsv is 32 */
max of brdv
> + continue;
> + break;
Having a conditional "continue" followed by a "break" looks strange.
if (brps <= 32) /* max of brdv is 32 */
break
> }
"brps" may be larger than 32 after the loop.
In the old code, the min_t() below handled that case.
> - k = min_t(int, k, ARRAY_SIZE(sh_msiof_spi_clk_table) - 1);
> -
> - sh_msiof_write(p, TSCR, sh_msiof_spi_clk_table[k].scr);
> + scr = sh_msiof_spi_div_table[k].brdv | (brps -1) << 8;
"scr = sh_msiof_spi_div_table[k].brdv | SCR_BRPS(i);" would avoid the need
for "- 1" and the hardcoded shift.
> + sh_msiof_write(p, TSCR, scr);
> if (!(p->chipdata->master_flags & SPI_MASTER_MUST_TX))
> - sh_msiof_write(p, RSCR, sh_msiof_spi_clk_table[k].scr);
> + sh_msiof_write(p, RSCR, scr);
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
