From: Alain Volmat <alain.volmat@st.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Wolfram Sang <wsa@the-dreams.de>,
linux-i2c@vger.kernel.org,
Pierre-Yves MORDRET <pierre-yves.mordret@st.com>,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
Alexandre Torgue <alexandre.torgue@st.com>
Subject: Re: [PATCH v3 4/6] i2c: stm32f7: switch to I²C generic property parsing
Date: Fri, 20 Mar 2020 09:23:42 +0100 [thread overview]
Message-ID: <20200320082342.GA30425@gnbcxd0016.gnb.st.com> (raw)
In-Reply-To: <20200316154929.20886-4-andriy.shevchenko@linux.intel.com>
On Mon, Mar 16, 2020 at 05:49:27PM +0200, Andy Shevchenko wrote:
> Switch to the new generic functions: i2c_parse_fw_timings().
>
> While here, replace hard coded values with standard bus frequency definitions.
>
> Cc: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v3: new patch instead of simple frequency conversion
> drivers/i2c/busses/i2c-stm32f7.c | 57 +++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
> index 6418f5982894..18f231f631f5 100644
> --- a/drivers/i2c/busses/i2c-stm32f7.c
> +++ b/drivers/i2c/busses/i2c-stm32f7.c
> @@ -344,9 +344,9 @@ struct stm32f7_i2c_dev {
> */
> static struct stm32f7_i2c_spec i2c_specs[] = {
> [STM32_I2C_SPEED_STANDARD] = {
> - .rate = 100000,
> - .rate_min = 80000,
> - .rate_max = 100000,
> + .rate = I2C_MAX_STANDARD_MODE_FREQ,
> + .rate_min = I2C_MAX_STANDARD_MODE_FREQ * 8 / 10, /* 80% */
> + .rate_max = I2C_MAX_STANDARD_MODE_FREQ,
> .fall_max = 300,
> .rise_max = 1000,
> .hddat_min = 0,
> @@ -356,9 +356,9 @@ static struct stm32f7_i2c_spec i2c_specs[] = {
> .h_min = 4000,
> },
> [STM32_I2C_SPEED_FAST] = {
> - .rate = 400000,
> - .rate_min = 320000,
> - .rate_max = 400000,
> + .rate = I2C_MAX_FAST_MODE_FREQ,
> + .rate_min = I2C_MAX_FAST_MODE_FREQ * 8 / 10, /* 80% */
> + .rate_max = I2C_MAX_FAST_MODE_FREQ,
> .fall_max = 300,
> .rise_max = 300,
> .hddat_min = 0,
> @@ -368,9 +368,9 @@ static struct stm32f7_i2c_spec i2c_specs[] = {
> .h_min = 600,
> },
> [STM32_I2C_SPEED_FAST_PLUS] = {
> - .rate = 1000000,
> - .rate_min = 800000,
> - .rate_max = 1000000,
> + .rate = I2C_MAX_FAST_MODE_PLUS_FREQ,
> + .rate_min = I2C_MAX_FAST_MODE_PLUS_FREQ * 8 / 10, /* 80% */
> + .rate_max = I2C_MAX_FAST_MODE_PLUS_FREQ,
> .fall_max = 100,
> .rise_max = 120,
> .hddat_min = 0,
> @@ -610,8 +610,25 @@ static int stm32f7_i2c_compute_timing(struct stm32f7_i2c_dev *i2c_dev,
> static int stm32f7_i2c_setup_timing(struct stm32f7_i2c_dev *i2c_dev,
> struct stm32f7_i2c_setup *setup)
> {
> + struct i2c_timings timings, *t = &timings;
> int ret = 0;
>
> + t->bus_freq_hz = I2C_MAX_STANDARD_MODE_FREQ;
> + t->scl_rise_ns = i2c_dev->setup.rise_time;
> + t->scl_fall_ns = i2c_dev->setup.fall_time;
> +
> + i2c_parse_fw_timings(&pdev->dev, t, false);
Andy, thanks for the patch.
Looks fine overall, apart from the above line which should be
i2c_parse_fw_timings(i2c_dev->dev, t, false);
> +
> + if (t->bus_freq_hz >= I2C_MAX_FAST_MODE_PLUS_FREQ)
> + i2c_dev->speed = STM32_I2C_SPEED_FAST_PLUS;
> + else if (t->bus_freq_hz >= I2C_MAX_FAST_MODE_FREQ)
> + i2c_dev->speed = STM32_I2C_SPEED_FAST;
> + else
> + i2c_dev->speed = STM32_I2C_SPEED_STANDARD;
> +
> + i2c_dev->setup.rise_time = t->scl_rise_ns;
> + i2c_dev->setup.fall_time = t->scl_fall_ns;
> +
> setup->speed = i2c_dev->speed;
> setup->speed_freq = i2c_specs[setup->speed].rate;
> setup->clock_src = clk_get_rate(i2c_dev->clk);
> @@ -1914,7 +1931,6 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
> struct stm32f7_i2c_dev *i2c_dev;
> const struct stm32f7_i2c_setup *setup;
> struct resource *res;
> - u32 clk_rate, rise_time, fall_time;
> struct i2c_adapter *adap;
> struct reset_control *rst;
> dma_addr_t phy_addr;
> @@ -1961,17 +1977,6 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
> return ret;
> }
>
> - i2c_dev->speed = STM32_I2C_SPEED_STANDARD;
> - ret = device_property_read_u32(&pdev->dev, "clock-frequency",
> - &clk_rate);
> - if (!ret && clk_rate >= 1000000) {
> - i2c_dev->speed = STM32_I2C_SPEED_FAST_PLUS;
> - } else if (!ret && clk_rate >= 400000) {
> - i2c_dev->speed = STM32_I2C_SPEED_FAST;
> - } else if (!ret && clk_rate >= 100000) {
> - i2c_dev->speed = STM32_I2C_SPEED_STANDARD;
> - }
> -
> rst = devm_reset_control_get(&pdev->dev, NULL);
> if (IS_ERR(rst)) {
> dev_err(&pdev->dev, "Error: Missing controller reset\n");
> @@ -2011,16 +2016,6 @@ static int stm32f7_i2c_probe(struct platform_device *pdev)
> }
> i2c_dev->setup = *setup;
>
> - ret = device_property_read_u32(i2c_dev->dev, "i2c-scl-rising-time-ns",
> - &rise_time);
> - if (!ret)
> - i2c_dev->setup.rise_time = rise_time;
> -
> - ret = device_property_read_u32(i2c_dev->dev, "i2c-scl-falling-time-ns",
> - &fall_time);
> - if (!ret)
> - i2c_dev->setup.fall_time = fall_time;
> -
> ret = stm32f7_i2c_setup_timing(i2c_dev, &i2c_dev->setup);
> if (ret)
> goto clk_free;
> --
> 2.25.1
>
next prev parent reply other threads:[~2020-03-20 8:23 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-16 15:49 [PATCH v3 1/6] i2c: core: Provide generic definitions for bus frequencies Andy Shevchenko
2020-03-16 15:49 ` [PATCH v3 2/6] i2c: core: Allow override timing properties with 0 Andy Shevchenko
2020-03-20 14:43 ` Wolfram Sang
2020-03-20 16:23 ` Andy Shevchenko
2020-03-20 16:44 ` Wolfram Sang
2020-03-20 16:46 ` Andy Shevchenko
2020-03-16 15:49 ` [PATCH v3 3/6] i2c: rcar: Consolidate timings calls in rcar_i2c_clock_calculate() Andy Shevchenko
2020-03-23 21:54 ` Wolfram Sang
2020-03-23 22:03 ` Andy Shevchenko
2020-03-24 8:13 ` Wolfram Sang
2020-03-24 9:02 ` Andy Shevchenko
2020-03-24 9:15 ` Wolfram Sang
2020-03-16 15:49 ` [PATCH v3 4/6] i2c: stm32f7: switch to I²C generic property parsing Andy Shevchenko
2020-03-20 8:23 ` Alain Volmat [this message]
2020-03-20 10:30 ` Andy Shevchenko
2020-03-16 15:49 ` [PATCH v3 5/6] i2c: algo: Use generic definitions for bus frequencies Andy Shevchenko
2020-03-19 17:19 ` [PATCH v3 1/6] i2c: core: Provide " Andy Shevchenko
2020-03-20 14:45 ` Wolfram Sang
2020-03-20 16:23 ` Andy Shevchenko
[not found] ` <20200316154929.20886-6-andriy.shevchenko@linux.intel.com>
2020-03-20 14:23 ` [PATCH v3 6/6] i2c: drivers: Use " Dmitry Osipenko
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=20200320082342.GA30425@gnbcxd0016.gnb.st.com \
--to=alain.volmat@st.com \
--cc=alexandre.torgue@st.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=linux-i2c@vger.kernel.org \
--cc=mcoquelin.stm32@gmail.com \
--cc=pierre-yves.mordret@st.com \
--cc=wsa@the-dreams.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).