All of lore.kernel.org
 help / color / mirror / Atom feed
From: Howard Mitchell <hm-n6SlHCM0l+K9FHfhHBbuYA@public.gmane.org>
To: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org,
	tiwai-l3A5Bk7waGM@public.gmane.org,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	perex-/Fr2/VpizcU@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] ASoC:pcm512x: Make PLL lock output selectable via device tree.
Date: Mon, 23 Mar 2015 09:43:59 +0000	[thread overview]
Message-ID: <550FE05F.6070204@hmbedded.co.uk> (raw)
In-Reply-To: <20150322162409.GG6643-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>

On 22/03/15 16:24, Mark Brown wrote:
> On Fri, Mar 20, 2015 at 09:22:43PM +0000, Howard Mitchell wrote:
>
>> +	if (pcm512x->pll_lock) {
>> +                if (of_property_read_u32(np, "pll-lock", &val) >= 0) {
>> +                        if (val > 6) {
>> +                                dev_err(dev, "Invalid pll-lock\n");
>> +                                ret = -EINVAL;
>> +                                goto err_clk;
>> +                        }
>> +                        pcm512x->pll_lock = val;
>> +                }
> This breaks existing boards which rely on GPIO 4 being set as the lock
> output.  This is very unfortunate since it's a silly thing for the
> driver to default to but nontheless we should really continue to support
> them - at a guess Peter's board is relying on this, and even if it isn't
> someone else's might.
I take your point, but the reason I pushed this patch was that I wanted 
to use GPIO4 for pll-out and unfortunately because the pll-lock 
configuration is after the pll-out configuration it stomps on it. If I 
modify the patch to provide a default for pll-lock I will then be 
obliged to specify pll-lock on another GPIO. The pcm5122 has limited IO 
so being forced to have a GPIO for pll-lock seems wrong to me. A future 
user of the device may well decide to use the GPIOs for other purposes 
and therefore not want a pll-lock signal at all. Surely we should allow 
for that possibility?

Given that Peter has indicated that he'd be happy with this solution and 
that this code hasn't reached a published kernel would it be reasonable 
to go ahead with my current patch (happy to clean up the indent issues 
that Peter pointed out of course)?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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: Howard Mitchell <hm@hmbedded.co.uk>
To: Mark Brown <broonie@kernel.org>
Cc: peda@axentia.se, tiwai@suse.de, lgirdwood@gmail.com,
	perex@perex.cz, robh+dt@kernel.org, pawel.moll@arm.com,
	mark.rutland@arm.com, ijc+devicetree@hellion.org.uk,
	galak@codeaurora.org, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ASoC:pcm512x: Make PLL lock output selectable via device tree.
Date: Mon, 23 Mar 2015 09:43:59 +0000	[thread overview]
Message-ID: <550FE05F.6070204@hmbedded.co.uk> (raw)
In-Reply-To: <20150322162409.GG6643@sirena.org.uk>

On 22/03/15 16:24, Mark Brown wrote:
> On Fri, Mar 20, 2015 at 09:22:43PM +0000, Howard Mitchell wrote:
>
>> +	if (pcm512x->pll_lock) {
>> +                if (of_property_read_u32(np, "pll-lock", &val) >= 0) {
>> +                        if (val > 6) {
>> +                                dev_err(dev, "Invalid pll-lock\n");
>> +                                ret = -EINVAL;
>> +                                goto err_clk;
>> +                        }
>> +                        pcm512x->pll_lock = val;
>> +                }
> This breaks existing boards which rely on GPIO 4 being set as the lock
> output.  This is very unfortunate since it's a silly thing for the
> driver to default to but nontheless we should really continue to support
> them - at a guess Peter's board is relying on this, and even if it isn't
> someone else's might.
I take your point, but the reason I pushed this patch was that I wanted 
to use GPIO4 for pll-out and unfortunately because the pll-lock 
configuration is after the pll-out configuration it stomps on it. If I 
modify the patch to provide a default for pll-lock I will then be 
obliged to specify pll-lock on another GPIO. The pcm5122 has limited IO 
so being forced to have a GPIO for pll-lock seems wrong to me. A future 
user of the device may well decide to use the GPIOs for other purposes 
and therefore not want a pll-lock signal at all. Surely we should allow 
for that possibility?

Given that Peter has indicated that he'd be happy with this solution and 
that this code hasn't reached a published kernel would it be reasonable 
to go ahead with my current patch (happy to clean up the indent issues 
that Peter pointed out of course)?

  parent reply	other threads:[~2015-03-23  9:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-20 21:22 [PATCH] ASoC:pcm512x: Make PLL lock output selectable via device tree Howard Mitchell
2015-03-22 16:24 ` Mark Brown
     [not found]   ` <20150322162409.GG6643-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-03-23  9:43     ` Howard Mitchell [this message]
2015-03-23  9:43       ` Howard Mitchell
     [not found]       ` <550FE05F.6070204-n6SlHCM0l+K9FHfhHBbuYA@public.gmane.org>
2015-03-23 11:00         ` Peter Rosin
2015-03-23 11:00           ` Peter Rosin
2015-03-23 11:14           ` Howard Mitchell
2015-03-23 11:14             ` Howard Mitchell
2015-03-23 16:56           ` Mark Brown
     [not found]             ` <20150323165621.GH14954-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-03-23 19:15               ` Howard Mitchell
2015-03-23 19:15                 ` Howard Mitchell
     [not found] ` <1426886563-10936-1-git-send-email-hm-n6SlHCM0l+K9FHfhHBbuYA@public.gmane.org>
2015-03-22 21:25   ` Peter Rosin
2015-03-22 21:25     ` Peter Rosin

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=550FE05F.6070204@hmbedded.co.uk \
    --to=hm-n6slhcm0l+k9fhfhhbbuya@public.gmane.org \
    --cc=alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
    --cc=lgirdwood-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org \
    --cc=perex-/Fr2/VpizcU@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=tiwai-l3A5Bk7waGM@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.