LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
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: Tue, 17 Mar 2015 13:02:08 +0200	[thread overview]
Message-ID: <1426590128.2330.25.camel@mm-sol.com> (raw)
In-Reply-To: <54FDEC36.5080403@codeaurora.org>

Hi, 

On Mon, 2015-03-09 at 11:53 -0700, Stephen Boyd wrote:
> On 03/09/15 01:20, Ivan T. Ivanov wrote:
> > 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.
> 
> Ouch. That sounds bad. Can you please add this information to the commit
> text?
> 
> > > > 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.

I will like to withdraw this patch. It fix the problem only
for CS lines, but misconfiguration could happen also for the
rest for the lines.

I will take a look in the pinctrl core code to see would it
be possible to print warning or something, when one driver
reconfigure pins already configured by another driver.

Regards,
Ivan  





  parent reply	other threads:[~2015-03-17 11:02 UTC|newest]

Thread overview: 13+ 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
2015-03-09  8:20     ` Ivan T. Ivanov
2015-03-09 18:53       ` Stephen Boyd
2015-03-10  8:31         ` Ivan T. Ivanov
2015-03-17 11:02         ` Ivan T. Ivanov [this message]
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
2015-03-10 11:06           ` Mark Brown
2015-03-10 12:53             ` Ivan T. Ivanov
2015-03-10 20:47               ` Mark Brown
2015-03-07 11:01 ` [PATCH 1/2] spi: qup: Fix cs-num DT property parsing 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=1426590128.2330.25.camel@mm-sol.com \
    --to=iivanov@mm-sol.com \
    --cc=broonie@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=sboyd@codeaurora.org \
    --subject='Re: [PATCH 2/2] spi: qup: Request CS GPIO'\''s during probe' \
    /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

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).