From: "Franklin S Cooper Jr." <fcooper@ti.com>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: "Ivan T. Ivanov" <iivanov.xz@gmail.com>,
Jarkko Nikula <jarkko.nikula@linux.intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
Mark Brown <broonie@kernel.org>, Sekhar Nori <nsekhar@ti.com>,
<ssantosh@kernel.org>, "Ivan T. Ivanov" <iivanov@mm-sol.com>,
<m-karicheri2@ti.com>,
Andy Shevchenko <andy.shevchenko@gmail.com>
Subject: Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect
Date: Thu, 15 Oct 2015 15:50:55 -0500 [thread overview]
Message-ID: <562011AF.5040306@ti.com> (raw)
In-Reply-To: <CAFSsGVtXP_Jd+CxvxfWCLXT_f+Tgv5SSZAbnBciqfr_BQMEZhA@mail.gmail.com>
On 10/14/2015 06:45 AM, Heiner Kallweit wrote:
> On Wed, Oct 14, 2015 at 1:08 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> +Cc: Jarkko to see from spi-pxa2xx prospective
>>
>> On Wed, Oct 14, 2015 at 12:47 PM, Ivan T. Ivanov <iivanov.xz@gmail.com> wrote:
>>> Adding Andy.
>>>
>>>
>>>> On Oct 13, 2015, at 12:01 AM, Franklin S Cooper Jr <fcooper@ti.com> wrote:
>>>>
>>>> Some devices depend on the master controller driver setup function being
>>>> called before calling any chipselect functions.
>>>>
>>>> Insure that this is done otherwise uninitialized structures may be
>>>> accessed causing a kernel panic.
>> As far as I understand my concern should be about spi-dw driver.
>>
>> So, I have just tested yesterday's linux-next with and without
>> proposed patch. Works for me:
>> Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>> ---
>>>> drivers/spi/spi.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>>> index 38006cc..9374d82 100644
>>>> --- a/drivers/spi/spi.c
>>>> +++ b/drivers/spi/spi.c
>>>> @@ -2053,11 +2053,11 @@ int spi_setup(struct spi_device *spi)
>>>> if (!spi->max_speed_hz)
>>>> spi->max_speed_hz = spi->master->max_speed_hz;
>>>>
>>>> - spi_set_cs(spi, false);
>>>> -
>>>> if (spi->master->setup)
>>>> status = spi->master->setup(spi);
>>>>
>>>> + spi_set_cs(spi, false);
>>>> +
>>>> dev_dbg(&spi->dev, "setup mode %d, %s%s%s%s%u bits/w, %u Hz max --> %d\n",
>>>> (int) (spi->mode & (SPI_CPOL | SPI_CPHA)),
>>>> (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
>>>> --
>>>> 2.6.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> The recent change to the bitbang driver leads to the the set_cs hook
> of spi_master being set
> now for all drivers using the bitbang layer. This hook is called also
> from spi_setup and therefore
> one possible side effect is issues with bitbang drivers implementing
> the chipselect hook of
> spi_bitbang with a dependency on the master being set up before.
> The proposed patch looks good to me.
> There should be no impact on drivers not using bitbang.
Thank all. Since nothing obvious seems wrong with this patch I will resend this patch without the RFC.
next prev parent reply other threads:[~2015-10-15 20:51 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-12 21:01 [RFC] spi: Switch call order of spi master setup and spi_set_cs Franklin S Cooper Jr
2015-10-12 21:01 ` [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect Franklin S Cooper Jr
2015-10-14 9:47 ` Ivan T. Ivanov
2015-10-14 11:08 ` Andy Shevchenko
2015-10-14 11:45 ` Heiner Kallweit
2015-10-15 20:50 ` Franklin S Cooper Jr. [this message]
2015-10-13 12:10 ` [RFC] spi: Switch call order of spi master setup and spi_set_cs Mark Brown
2015-10-13 14:03 ` Franklin S Cooper Jr.
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=562011AF.5040306@ti.com \
--to=fcooper@ti.com \
--cc=andy.shevchenko@gmail.com \
--cc=broonie@kernel.org \
--cc=hkallweit1@gmail.com \
--cc=iivanov.xz@gmail.com \
--cc=iivanov@mm-sol.com \
--cc=jarkko.nikula@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-spi@vger.kernel.org \
--cc=m-karicheri2@ti.com \
--cc=nsekhar@ti.com \
--cc=ssantosh@kernel.org \
/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