All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ivan T. Ivanov" <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
To: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 2/2] spi: qup: Request CS GPIO's during probe
Date: Mon, 09 Mar 2015 10:20:07 +0200	[thread overview]
Message-ID: <1425889207.2440.9.camel@mm-sol.com> (raw)
In-Reply-To: <54F9F31A.3050501-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>


Hi Stephen,

> On Mar 6, 2015, at 8:34 PM, Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On 03/06/15 07:26, Ivan T. Ivanov wrote:
> > Ensure that driver is owner of the GPIO's used for CS signals.
> Why? What happens if we don’t?

We can have wrong DT configuration, which could reconfigure
GPIO’s without any warning or error.

> > Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
> > ---
> > drivers/spi/spi-qup.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
> > index 2b2c359..a07ba46 100644
> > --- a/drivers/spi/spi-qup.c
> > +++ b/drivers/spi/spi-qup.c
> > @@ -14,11 +14,13 @@
> > #include <linux/clk.h>
> > #include <linux/delay.h>
> > #include <linux/err.h>
> > +#include <linux/gpio.h>
> > #include <linux/interrupt.h>
> > #include <linux/io.h>
> > #include <linux/list.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > +#include <linux/of_gpio.h>
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/spi/spi.h>
> > @@ -499,7 +501,7 @@ static int spi_qup_probe(struct platform_device *pdev)
> >         struct device *dev;
> >         void __iomem *base;
> >         u32 max_freq, iomode, num_cs;
> > -       int ret, irq, size;
> > +       int ret, irq, size, cs, cs_gpio;
> > 
> >         dev = &pdev->dev;
> >         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > @@ -556,6 +558,19 @@ static int spi_qup_probe(struct platform_device *pdev)
> >         else
> >                 master->num_chipselect = num_cs;
> > 
> > +       for (cs = 0; cs < master->num_chipselect; cs++) {
> > +               cs_gpio = of_get_named_gpio(dev->of_node, "cs-gpios", cs);
> > +
> > +               if (!gpio_is_valid(cs_gpio))
> > +                       continue;
> > +
> > +               ret = devm_gpio_request(&pdev->dev, cs_gpio, "spi-qup-cs");
> > +               if (ret) {
> > +                       dev_err(&pdev->dev, "can't get cs gpios\n");
> > +                       goto error;
> > +               }
> > +       }
> > +
> >         master->bus_num = pdev->id;
> Is this related to [1]? In that case I was just relying on DT/pinctrl to
> properly request the gpios.

But the DT/pinctrl did not request GPIO’s, it just configure them.
For some reason we are ending without any pinctrl_map of type 
PIN_MAP_TYPE_MUX_GROUP, which is used for pin reservation. 

> [1] https://lkml.org/lkml/2014/5/27/784

I have missed this one. I have considered same approach, but abandoned
it, because of issues related to double request reasons mentioned by Mark.

Regards,
Ivan
--
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: "Ivan T. Ivanov" <iivanov@mm-sol.com>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: Mark Brown <broonie@kernel.org>,
	linus.walleij@linaro.org, linux-spi@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 2/2] spi: qup: Request CS GPIO's during probe
Date: Mon, 09 Mar 2015 10:20:07 +0200	[thread overview]
Message-ID: <1425889207.2440.9.camel@mm-sol.com> (raw)
In-Reply-To: <54F9F31A.3050501@codeaurora.org>


Hi Stephen,

> On Mar 6, 2015, at 8:34 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 03/06/15 07:26, Ivan T. Ivanov wrote:
> > Ensure that driver is owner of the GPIO's used for CS signals.
> Why? What happens if we don’t?

We can have wrong DT configuration, which could reconfigure
GPIO’s without any warning or error.

> > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> > ---
> > drivers/spi/spi-qup.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
> > index 2b2c359..a07ba46 100644
> > --- a/drivers/spi/spi-qup.c
> > +++ b/drivers/spi/spi-qup.c
> > @@ -14,11 +14,13 @@
> > #include <linux/clk.h>
> > #include <linux/delay.h>
> > #include <linux/err.h>
> > +#include <linux/gpio.h>
> > #include <linux/interrupt.h>
> > #include <linux/io.h>
> > #include <linux/list.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > +#include <linux/of_gpio.h>
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/spi/spi.h>
> > @@ -499,7 +501,7 @@ static int spi_qup_probe(struct platform_device *pdev)
> >         struct device *dev;
> >         void __iomem *base;
> >         u32 max_freq, iomode, num_cs;
> > -       int ret, irq, size;
> > +       int ret, irq, size, cs, cs_gpio;
> > 
> >         dev = &pdev->dev;
> >         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > @@ -556,6 +558,19 @@ static int spi_qup_probe(struct platform_device *pdev)
> >         else
> >                 master->num_chipselect = num_cs;
> > 
> > +       for (cs = 0; cs < master->num_chipselect; cs++) {
> > +               cs_gpio = of_get_named_gpio(dev->of_node, "cs-gpios", cs);
> > +
> > +               if (!gpio_is_valid(cs_gpio))
> > +                       continue;
> > +
> > +               ret = devm_gpio_request(&pdev->dev, cs_gpio, "spi-qup-cs");
> > +               if (ret) {
> > +                       dev_err(&pdev->dev, "can't get cs gpios\n");
> > +                       goto error;
> > +               }
> > +       }
> > +
> >         master->bus_num = pdev->id;
> Is this related to [1]? In that case I was just relying on DT/pinctrl to
> properly request the gpios.

But the DT/pinctrl did not request GPIO’s, it just configure them.
For some reason we are ending without any pinctrl_map of type 
PIN_MAP_TYPE_MUX_GROUP, which is used for pin reservation. 

> [1] https://lkml.org/lkml/2014/5/27/784

I have missed this one. I have considered same approach, but abandoned
it, because of issues related to double request reasons mentioned by Mark.

Regards,
Ivan

  parent reply	other threads:[~2015-03-09  8:20 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-06 15:26 [PATCH 1/2] spi: qup: Fix cs-num DT property parsing Ivan T. Ivanov
2015-03-06 15:26 ` [PATCH 2/2] spi: qup: Request CS GPIO's during probe Ivan T. Ivanov
2015-03-06 18:34   ` Stephen Boyd
     [not found]     ` <54F9F31A.3050501-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2015-03-09  8:20       ` Ivan T. Ivanov [this message]
2015-03-09  8:20         ` Ivan T. Ivanov
2015-03-09 18:53         ` Stephen Boyd
     [not found]           ` <54FDEC36.5080403-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2015-03-10  8:31             ` Ivan T. Ivanov
2015-03-10  8:31               ` Ivan T. Ivanov
2015-03-17 11:02           ` Ivan T. Ivanov
     [not found]   ` <1425655578-22400-2-git-send-email-iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
2015-03-07 10:59     ` Mark Brown
2015-03-07 10:59       ` Mark Brown
     [not found]       ` <1425889415.2440.10.camel@mm-sol.com>
     [not found]         ` <20150309182841.GW28806@sirena.org.uk>
2015-03-10  8:10           ` Ivan T. Ivanov
     [not found]             ` <1425975056.2440.24.camel-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
2015-03-10 11:06               ` Mark Brown
2015-03-10 11:06                 ` Mark Brown
2015-03-10 12:53                 ` Ivan T. Ivanov
     [not found]                   ` <1425991997.12581.1.camel-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
2015-03-10 20:47                     ` Mark Brown
2015-03-10 20:47                       ` Mark Brown
     [not found] ` <1425655578-22400-1-git-send-email-iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
2015-03-07 11:01   ` [PATCH 1/2] spi: qup: Fix cs-num DT property parsing Mark Brown
2015-03-07 11:01     ` Mark Brown

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=1425889207.2440.9.camel@mm-sol.com \
    --to=iivanov-neyub+7iv8pqt0dzr+alfa@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=sboyd-sgV2jX0FEOL9JmXXK+q4OQ@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.