All of lore.kernel.org
 help / color / mirror / Atom feed
From: Flavio Suligoi <f.suligoi@asem.it>
To: Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	Daniel Mack <daniel@zonque.org>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	"Robert Jarzmik" <robert.jarzmik@free.fr>,
	Mark Brown <broonie@kernel.org>
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 1/2] spi: pxa2xx: fix SCR (divisor) calculation
Date: Thu, 11 Apr 2019 13:10:04 +0000	[thread overview]
Message-ID: <3ceba8bc20f8496a9c72653fd450d729@asem.it> (raw)
In-Reply-To: <34b8442a-15ac-dea2-a088-fde8b1966e1e@linux.intel.com>

Hi Jarkko,

> > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
> > index f7068cc..c9560a1 100644
> > --- a/drivers/spi/spi-pxa2xx.c
> > +++ b/drivers/spi/spi-pxa2xx.c
> > @@ -884,10 +884,15 @@ static unsigned int ssp_get_clk_div(struct
> driver_data *drv_data, int rate)
> >
> >   	rate = min_t(int, ssp_clk, rate);
> >
> > +	/*
> > +	 * Calculate the divisor for the SCR (Serial Clock Rate), avoiding
> > +	 * that the SSP transmission rate can be greater than the device
> rate
> > +	 */
> >   	if (ssp->type == PXA25x_SSP || ssp->type == CE4100_SSP)
> > -		return (ssp_clk / (2 * rate) - 1) & 0xff;
> > +		return (ssp_clk / (2 * rate) - 1 +
> > +			(ssp_clk % (2 * rate) ? 1 : 0)) & 0xff;
> >   	else
> > -		return (ssp_clk / rate - 1) & 0xfff;
> > +		return (ssp_clk / rate - 1 + (ssp_clk % rate ? 1 : 0)) &
> 0xfff;
> >   }
> >
> I think DIV_ROUND_UP() - 1 would also fix this?
> 
> I realized we have also another issue here with the low rates.
> Calculated divider will underflow due masking with 0xff or 0xfff when
> the rate is low enough.
> 
> Would you want to fix that by setting the ctlr->min_speed_hz so that spi
> core can validate the rate? I'm asking since it goes well together with
> your fix. Maybe one patch setting the min_speed_hz and getting masking
> off from here and then another fixing the rate calculation.

Ok for the two separate patches, I prepare them as soon as I can.

Thanks,

Flavio

WARNING: multiple messages have this Message-ID (diff)
From: Flavio Suligoi <f.suligoi@asem.it>
To: Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	Daniel Mack <daniel@zonque.org>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	"Robert Jarzmik" <robert.jarzmik@free.fr>,
	Mark Brown <broonie@kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>
Subject: RE: [PATCH 1/2] spi: pxa2xx: fix SCR (divisor) calculation
Date: Thu, 11 Apr 2019 13:10:04 +0000	[thread overview]
Message-ID: <3ceba8bc20f8496a9c72653fd450d729@asem.it> (raw)
In-Reply-To: <34b8442a-15ac-dea2-a088-fde8b1966e1e@linux.intel.com>

Hi Jarkko,

> > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
> > index f7068cc..c9560a1 100644
> > --- a/drivers/spi/spi-pxa2xx.c
> > +++ b/drivers/spi/spi-pxa2xx.c
> > @@ -884,10 +884,15 @@ static unsigned int ssp_get_clk_div(struct
> driver_data *drv_data, int rate)
> >
> >   	rate = min_t(int, ssp_clk, rate);
> >
> > +	/*
> > +	 * Calculate the divisor for the SCR (Serial Clock Rate), avoiding
> > +	 * that the SSP transmission rate can be greater than the device
> rate
> > +	 */
> >   	if (ssp->type == PXA25x_SSP || ssp->type == CE4100_SSP)
> > -		return (ssp_clk / (2 * rate) - 1) & 0xff;
> > +		return (ssp_clk / (2 * rate) - 1 +
> > +			(ssp_clk % (2 * rate) ? 1 : 0)) & 0xff;
> >   	else
> > -		return (ssp_clk / rate - 1) & 0xfff;
> > +		return (ssp_clk / rate - 1 + (ssp_clk % rate ? 1 : 0)) &
> 0xfff;
> >   }
> >
> I think DIV_ROUND_UP() - 1 would also fix this?
> 
> I realized we have also another issue here with the low rates.
> Calculated divider will underflow due masking with 0xff or 0xfff when
> the rate is low enough.
> 
> Would you want to fix that by setting the ctlr->min_speed_hz so that spi
> core can validate the rate? I'm asking since it goes well together with
> your fix. Maybe one patch setting the min_speed_hz and getting masking
> off from here and then another fixing the rate calculation.

Ok for the two separate patches, I prepare them as soon as I can.

Thanks,

Flavio
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-04-11 13:10 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-10 12:51 [PATCH 1/2] spi: pxa2xx: fix SCR (divisor) calculation Flavio Suligoi
2019-04-10 12:51 ` Flavio Suligoi
2019-04-10 12:51 ` [PATCH 2/2] spi: pxa2xx: use a module softdep for dw_dmac Flavio Suligoi
2019-04-10 12:51   ` Flavio Suligoi
2019-04-10 12:56   ` Mark Brown
2019-04-10 12:56     ` Mark Brown
2019-04-10 14:05     ` Flavio Suligoi
2019-04-10 14:05       ` Flavio Suligoi
2019-04-10 15:28       ` Mark Brown
2019-04-10 15:28         ` Mark Brown
2019-04-11  7:14         ` Flavio Suligoi
2019-04-11  7:14           ` Flavio Suligoi
2019-04-11 10:42           ` Mark Brown
2019-04-11 10:42             ` Mark Brown
2019-04-11 11:31             ` Flavio Suligoi
2019-04-11 11:31               ` Flavio Suligoi
2019-04-12  8:53   ` Applied "spi: pxa2xx: use a module softdep for dw_dmac" to the spi tree Mark Brown
2019-04-12  8:53     ` Mark Brown
2019-04-12  8:53     ` Mark Brown
2019-05-02  2:19   ` Mark Brown
2019-05-02  2:19     ` Mark Brown
2019-05-02  2:19     ` Mark Brown
2019-04-11 11:55 ` [PATCH 1/2] spi: pxa2xx: fix SCR (divisor) calculation Jarkko Nikula
2019-04-11 11:55   ` Jarkko Nikula
2019-04-11 13:10   ` Flavio Suligoi [this message]
2019-04-11 13:10     ` Flavio Suligoi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3ceba8bc20f8496a9c72653fd450d729@asem.it \
    --to=f.suligoi@asem.it \
    --cc=broonie@kernel.org \
    --cc=daniel@zonque.org \
    --cc=haojian.zhuang@gmail.com \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=robert.jarzmik@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.