All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Nikula <jarkko.nikula@linux.intel.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	mika.westerberg@linux.intel.com, wsa@the-dreams.de,
	linux-i2c@vger.kernel.org,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] i2c: designware: don't infer timings described by ACPI from clock rate
Date: Fri, 19 May 2017 13:37:23 +0300	[thread overview]
Message-ID: <24ea1db0-e352-6a4e-ab67-41b537351c1f@linux.intel.com> (raw)
In-Reply-To: <CAKv+Gu-wRmxWnT+5mb19h2Ggq5kS8qU9dMbEgoPK=0b06P0bFw@mail.gmail.com>

On 05/19/2017 01:06 PM, Ard Biesheuvel wrote:
> On 19 May 2017 at 11:01, Jarkko Nikula <jarkko.nikula@linux.intel.com> wrote:
>> On 05/19/2017 11:56 AM, Ard Biesheuvel wrote:
>>>
>>> Commit bd698d24b1b57 ("i2c: designware: Get selected speed mode
>>> sda-hold-time via ACPI") updated the logic that reads the timing
>>> parameters for various I2C bus rates from the DSDT, to only read
>>> the timing parameters for the currently selected mode.
>>>
>>> This causes a WARN_ON() splat on platforms that legally omit the clock
>>> frequency from the ACPI description, because in the new situation, the
>>> core I2C designware driver still accesses the fields in the driver
>>> struct that we no longer populate, and proceeds to calculate them from
>>> the clock frequency. Since the clock frequency is unspecified, the
>>> driver complains loudly using a WARN_ON().
>>>
>>> So revert back to the old situation, where the struct fields for all
>>> timings are populated, but retain the new logic which chooses the SDA
>>> hold time from the timing mode that is currently in use.
>>>
>>> Fixes: bd698d24b1b57 ("i2c: designware: Get selected speed mode ...")
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  drivers/i2c/busses/i2c-designware-platdrv.c | 18 ++++++++++--------
>>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>> Thanks, this is ok to me. Let's add also kudos to Lorenzo:
>>
>> Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>
> Thanks. I suppose this is going in as a fix? If not, please cc to -stable
>
That's not needed as the bd698d24b1b57 came during pre 4.12-rc1 cycle. 
But good to get this into 4.12 due probable regression in high-speed 
transfers and to get rid of log spamming.

-- 
Jarkko

WARNING: multiple messages have this Message-ID (diff)
From: jarkko.nikula@linux.intel.com (Jarkko Nikula)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] i2c: designware: don't infer timings described by ACPI from clock rate
Date: Fri, 19 May 2017 13:37:23 +0300	[thread overview]
Message-ID: <24ea1db0-e352-6a4e-ab67-41b537351c1f@linux.intel.com> (raw)
In-Reply-To: <CAKv+Gu-wRmxWnT+5mb19h2Ggq5kS8qU9dMbEgoPK=0b06P0bFw@mail.gmail.com>

On 05/19/2017 01:06 PM, Ard Biesheuvel wrote:
> On 19 May 2017 at 11:01, Jarkko Nikula <jarkko.nikula@linux.intel.com> wrote:
>> On 05/19/2017 11:56 AM, Ard Biesheuvel wrote:
>>>
>>> Commit bd698d24b1b57 ("i2c: designware: Get selected speed mode
>>> sda-hold-time via ACPI") updated the logic that reads the timing
>>> parameters for various I2C bus rates from the DSDT, to only read
>>> the timing parameters for the currently selected mode.
>>>
>>> This causes a WARN_ON() splat on platforms that legally omit the clock
>>> frequency from the ACPI description, because in the new situation, the
>>> core I2C designware driver still accesses the fields in the driver
>>> struct that we no longer populate, and proceeds to calculate them from
>>> the clock frequency. Since the clock frequency is unspecified, the
>>> driver complains loudly using a WARN_ON().
>>>
>>> So revert back to the old situation, where the struct fields for all
>>> timings are populated, but retain the new logic which chooses the SDA
>>> hold time from the timing mode that is currently in use.
>>>
>>> Fixes: bd698d24b1b57 ("i2c: designware: Get selected speed mode ...")
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  drivers/i2c/busses/i2c-designware-platdrv.c | 18 ++++++++++--------
>>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>> Thanks, this is ok to me. Let's add also kudos to Lorenzo:
>>
>> Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>
> Thanks. I suppose this is going in as a fix? If not, please cc to -stable
>
That's not needed as the bd698d24b1b57 came during pre 4.12-rc1 cycle. 
But good to get this into 4.12 due probable regression in high-speed 
transfers and to get rid of log spamming.

-- 
Jarkko

  reply	other threads:[~2017-05-19 10:37 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-19  8:56 [PATCH] i2c: designware: don't infer timings described by ACPI from clock rate Ard Biesheuvel
2017-05-19  8:56 ` Ard Biesheuvel
2017-05-19 10:01 ` Jarkko Nikula
2017-05-19 10:01   ` Jarkko Nikula
2017-05-19 10:06   ` Ard Biesheuvel
2017-05-19 10:06     ` Ard Biesheuvel
2017-05-19 10:37     ` Jarkko Nikula [this message]
2017-05-19 10:37       ` Jarkko Nikula
2017-05-19 10:48       ` Ard Biesheuvel
2017-05-19 10:48         ` Ard Biesheuvel
2017-05-19 12:38 ` Wolfram Sang
2017-05-19 12:38   ` Wolfram Sang
2017-05-21  8:09 ` Ard Biesheuvel
2017-05-21  8:09   ` Ard Biesheuvel
2017-05-22  5:44   ` Jan Kiszka
2017-05-22  5:44     ` Jan Kiszka
2017-05-22  6:35     ` Jarkko Nikula
2017-05-22  6:35       ` Jarkko Nikula
2017-05-22  5:46   ` [PATCH] i2c: designware: Fix bogus sda_hold_time due to uninitialized vars Jan Kiszka
2017-05-22  5:46     ` Jan Kiszka
2017-05-22  5:46     ` Jan Kiszka
2017-05-22  6:02     ` Ard Biesheuvel
2017-05-22  6:02       ` Ard Biesheuvel
2017-05-22  6:37     ` Jarkko Nikula
2017-05-22  6:37       ` Jarkko Nikula
2017-05-22  8:36     ` Wolfram Sang
2017-05-22  8:36       ` Wolfram Sang

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=24ea1db0-e352-6a4e-ab67-41b537351c1f@linux.intel.com \
    --to=jarkko.nikula@linux.intel.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=mika.westerberg@linux.intel.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 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.