From: "Franklin S Cooper Jr." <fcooper-l0cyMroinI0@public.gmane.org>
To: Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: "Ivan T. Ivanov"
<iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
Jarkko Nikula
<jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
"linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>,
<ssantosh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
"Ivan T. Ivanov"
<iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>,
<m-karicheri2-l0cyMroinI0@public.gmane.org>,
Andy Shevchenko
<andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
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-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 10/14/2015 06:45 AM, Heiner Kallweit wrote:
> On Wed, Oct 14, 2015 at 1:08 PM, Andy Shevchenko
> <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> +Cc: Jarkko to see from spi-pxa2xx prospective
>>
>> On Wed, Oct 14, 2015 at 12:47 PM, Ivan T. Ivanov <iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Adding Andy.
>>>
>>>
>>>> On Oct 13, 2015, at 12:01 AM, Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>>>> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>>>> ---
>>>> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
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:50 UTC|newest]
Thread overview: 14+ 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 ` 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
[not found] ` <1444683671-15570-2-git-send-email-fcooper-l0cyMroinI0@public.gmane.org>
2015-10-14 9:47 ` Ivan T. Ivanov
2015-10-14 9:47 ` Ivan T. Ivanov
[not found] ` <32A34513-E312-40FD-9390-B080A85822B7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-10-14 11:08 ` Andy Shevchenko
2015-10-14 11:08 ` Andy Shevchenko
2015-10-14 11:45 ` Heiner Kallweit
[not found] ` <CAFSsGVtXP_Jd+CxvxfWCLXT_f+Tgv5SSZAbnBciqfr_BQMEZhA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-10-15 20:50 ` Franklin S Cooper Jr. [this message]
2015-10-15 20:50 ` Franklin S Cooper Jr.
2015-10-28 0:52 ` Applied "spi: Setup the master controller driver before setting the chipselect" to the spi tree Mark Brown
[not found] ` <1444683671-15570-1-git-send-email-fcooper-l0cyMroinI0@public.gmane.org>
2015-10-13 12:10 ` [RFC] spi: Switch call order of spi master setup and spi_set_cs Mark Brown
2015-10-13 12:10 ` 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-l0cymroini0@public.gmane.org \
--cc=andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org \
--cc=iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=m-karicheri2-l0cyMroinI0@public.gmane.org \
--cc=nsekhar-l0cyMroinI0@public.gmane.org \
--cc=ssantosh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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 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.