All of lore.kernel.org
 help / color / mirror / Atom feed
From: linux@roeck-us.net (Guenter Roeck)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] clk: si570: Add a driver for SI570 oscillators
Date: Thu, 19 Sep 2013 14:11:12 -0700	[thread overview]
Message-ID: <20130919211112.GA11537@roeck-us.net> (raw)
In-Reply-To: <86ee8eea-7a98-443b-a156-46f3db1f9678@CO1EHSMHS031.ehs.local>

On Thu, Sep 19, 2013 at 01:59:38PM -0700, S?ren Brinkmann wrote:
> On Thu, Sep 19, 2013 at 09:44:38AM -0700, Guenter Roeck wrote:
> > On Thu, Sep 19, 2013 at 09:01:01AM -0700, S?ren Brinkmann wrote:
> > > On Thu, Sep 19, 2013 at 06:17:12AM -0700, Guenter Roeck wrote:
> > > > On Wed, Sep 18, 2013 at 03:43:38PM -0700, Soren Brinkmann wrote:
> [...]
> > > > > +	if (of_property_read_bool(client->dev.of_node,
> > > > > +				"temperature-stability-7ppm"))
> > > > > +		data->div_offset = SI570_DIV_OFFSET_7PPM;
> > > > > +
> > > > Just noticed that you dropped platform data support. Doesn't matter much for me
> > > > right now, but in my previous company we used the chip on an x86 system which
> > > > does not support devicetree. Would be nice to keep it and derive platform data
> > > > from devicetree data if provided, like other drivers do it.
> > > I'll look into this. The issue I have with that is, I can hardly test it
> > > since we only use this on Zynq which uses DT. So, I'd rather prefer to
> > > not include it unless somebody volunteers to test it.
> > > 
> > Fair enough. I can not test it myself anymore, and my previous employer
> > now has a strict non-contributions-to-linux policy, so I guess they won't
> > test it either or at least not publish any test results. Leave it out.
> > 
> > > > The 7ppm option is only relevant for si570/si751 and not supported on
> > > > si598/si599. You should mention that in the bindings document and check for it.
> > > Right, I'll add a note in the doc. And ignore it for devices this does
> > > not apply.
> > > 
> > I would bail out, but that is your call.
> Correct me if I'm wrong, but in general drivers do not test for
> unsupported properties. So, in case we have one of the supported 59x
> device, we should not test whether a (unsupported) property is present,
> just to fail in that case, IMHO.
> 
Ok with me if that is the accepted way of handling this condition.

Guenter

WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <linux@roeck-us.net>
To: "Sören Brinkmann" <soren.brinkmann@xilinx.com>
Cc: Rob Herring <rob.herring@calxeda.com>,
	Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Rob Landley <rob@landley.net>,
	Mike Turquette <mturquette@linaro.org>,
	Grant Likely <grant.likely@linaro.org>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Hyun Kwon <hyunk@xilinx.com>
Subject: Re: [PATCH v2] clk: si570: Add a driver for SI570 oscillators
Date: Thu, 19 Sep 2013 14:11:12 -0700	[thread overview]
Message-ID: <20130919211112.GA11537@roeck-us.net> (raw)
In-Reply-To: <86ee8eea-7a98-443b-a156-46f3db1f9678@CO1EHSMHS031.ehs.local>

On Thu, Sep 19, 2013 at 01:59:38PM -0700, Sören Brinkmann wrote:
> On Thu, Sep 19, 2013 at 09:44:38AM -0700, Guenter Roeck wrote:
> > On Thu, Sep 19, 2013 at 09:01:01AM -0700, Sören Brinkmann wrote:
> > > On Thu, Sep 19, 2013 at 06:17:12AM -0700, Guenter Roeck wrote:
> > > > On Wed, Sep 18, 2013 at 03:43:38PM -0700, Soren Brinkmann wrote:
> [...]
> > > > > +	if (of_property_read_bool(client->dev.of_node,
> > > > > +				"temperature-stability-7ppm"))
> > > > > +		data->div_offset = SI570_DIV_OFFSET_7PPM;
> > > > > +
> > > > Just noticed that you dropped platform data support. Doesn't matter much for me
> > > > right now, but in my previous company we used the chip on an x86 system which
> > > > does not support devicetree. Would be nice to keep it and derive platform data
> > > > from devicetree data if provided, like other drivers do it.
> > > I'll look into this. The issue I have with that is, I can hardly test it
> > > since we only use this on Zynq which uses DT. So, I'd rather prefer to
> > > not include it unless somebody volunteers to test it.
> > > 
> > Fair enough. I can not test it myself anymore, and my previous employer
> > now has a strict non-contributions-to-linux policy, so I guess they won't
> > test it either or at least not publish any test results. Leave it out.
> > 
> > > > The 7ppm option is only relevant for si570/si751 and not supported on
> > > > si598/si599. You should mention that in the bindings document and check for it.
> > > Right, I'll add a note in the doc. And ignore it for devices this does
> > > not apply.
> > > 
> > I would bail out, but that is your call.
> Correct me if I'm wrong, but in general drivers do not test for
> unsupported properties. So, in case we have one of the supported 59x
> device, we should not test whether a (unsupported) property is present,
> just to fail in that case, IMHO.
> 
Ok with me if that is the accepted way of handling this condition.

Guenter

  reply	other threads:[~2013-09-19 21:11 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-18 22:43 [PATCH v2] clk: si570: Add a driver for SI570 oscillators Soren Brinkmann
2013-09-18 22:43 ` Soren Brinkmann
2013-09-18 22:43 ` Soren Brinkmann
2013-09-18 22:43 ` Soren Brinkmann
2013-09-18 22:43   ` Soren Brinkmann
2013-09-18 22:43   ` Soren Brinkmann
2013-09-18 22:47   ` Sören Brinkmann
2013-09-18 22:47     ` Sören Brinkmann
2013-09-18 22:47     ` Sören Brinkmann
2013-09-18 23:02 ` Joe Perches
2013-09-18 23:02   ` Joe Perches
2013-09-18 23:02   ` Joe Perches
2013-09-18 23:09   ` Sören Brinkmann
2013-09-18 23:09     ` Sören Brinkmann
2013-09-18 23:09     ` Sören Brinkmann
2013-09-18 23:18     ` Joe Perches
2013-09-18 23:18       ` Joe Perches
2013-09-18 23:18       ` Joe Perches
2013-09-18 23:32       ` Sören Brinkmann
2013-09-18 23:32         ` Sören Brinkmann
2013-09-18 23:32         ` Sören Brinkmann
2013-09-19  0:23         ` Guenter Roeck
2013-09-19  0:23           ` Guenter Roeck
2013-09-19 14:48           ` Sören Brinkmann
2013-09-19 14:48             ` Sören Brinkmann
2013-09-19 14:48             ` Sören Brinkmann
2013-09-19 13:17 ` Guenter Roeck
2013-09-19 13:17   ` Guenter Roeck
2013-09-19 16:01   ` Sören Brinkmann
2013-09-19 16:01     ` Sören Brinkmann
2013-09-19 16:01     ` Sören Brinkmann
2013-09-19 16:44     ` Guenter Roeck
2013-09-19 16:44       ` Guenter Roeck
2013-09-19 20:59       ` Sören Brinkmann
2013-09-19 20:59         ` Sören Brinkmann
2013-09-19 20:59         ` Sören Brinkmann
2013-09-19 21:11         ` Guenter Roeck [this message]
2013-09-19 21:11           ` Guenter Roeck
2013-09-19 18:05 ` Mike Turquette
2013-09-19 18:05   ` Mike Turquette
2013-09-19 18:05   ` Mike Turquette
2013-09-19 18:40   ` Sören Brinkmann
2013-09-19 18:40     ` Sören Brinkmann
2013-09-19 18:40     ` Sören Brinkmann
2013-09-19 21:15   ` Guenter Roeck
2013-09-19 21:15     ` Guenter Roeck
2013-09-19 21:15     ` Guenter Roeck
2013-09-19 21:57     ` Sören Brinkmann
2013-09-19 21:57       ` Sören Brinkmann
2013-09-19 21:57       ` Sören Brinkmann

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=20130919211112.GA11537@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=linux-arm-kernel@lists.infradead.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.