All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Lee Jones <lee.jones@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	linus.walleij@stericsson.com, arnd@arndb.de,
	linux-kernel@vger.kernel.org, grant.likely@secretlab.ca,
	linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 09/14] i2c: Add Device Tree support to the Nomadik I2C driver
Date: Fri, 15 Jun 2012 10:32:32 +0100	[thread overview]
Message-ID: <20120615093231.GB3898@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <4FDA341C.8010501@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]

On Thu, Jun 14, 2012 at 07:57:32PM +0100, Lee Jones wrote:
> On 14/06/12 19:36, Mark Brown wrote:

> >You're not understanding Linus' point.  The compatible string isn't
> >useful here because properties like the maximum clock rate of the bus
> >depend on the board design, not the silicon.  The controller may be
> >perfectly happy to run at a given rate but other devices on the bus or
> >the electrical engineering of the PCB itself may restrict this further.

> And you're not understanding mine. ;)

I think we are (or at least I do)...

> You can have multiple compatible strings for a single driver. Which
> one you reference from the Device Tree will dictate which group of
> settings are used, including variation of clock rates.

I'm certainly well aware of that.  The problem is that the compatible
strings should identify a particular IP or piece of silicon and be
unchanging properties of that hardware - for example, working out how
big a FIFO in a device is from the compatible information is totally
sensible and reasonable.  Things like the I2C bus clock speed on the
other hand depend not on the individual device but on how it's been
integrated into the system so you'd end up saying things like
"nomadik-i2c-100khz-bus" in the compatible string which isn't great.  

If there's something like variation in the maximum speed that can be
supported by the different silicon the driver handles then working that
out from compatible and using it as the default is sensible but that's
not the same thing as providing customisation of the system bus
frequency to the user and if there's only one of those two that's
configurable it should be the system bus frequency.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
To: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Linus Walleij
	<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 09/14] i2c: Add Device Tree support to the Nomadik I2C driver
Date: Fri, 15 Jun 2012 10:32:32 +0100	[thread overview]
Message-ID: <20120615093231.GB3898@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <4FDA341C.8010501-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]

On Thu, Jun 14, 2012 at 07:57:32PM +0100, Lee Jones wrote:
> On 14/06/12 19:36, Mark Brown wrote:

> >You're not understanding Linus' point.  The compatible string isn't
> >useful here because properties like the maximum clock rate of the bus
> >depend on the board design, not the silicon.  The controller may be
> >perfectly happy to run at a given rate but other devices on the bus or
> >the electrical engineering of the PCB itself may restrict this further.

> And you're not understanding mine. ;)

I think we are (or at least I do)...

> You can have multiple compatible strings for a single driver. Which
> one you reference from the Device Tree will dictate which group of
> settings are used, including variation of clock rates.

I'm certainly well aware of that.  The problem is that the compatible
strings should identify a particular IP or piece of silicon and be
unchanging properties of that hardware - for example, working out how
big a FIFO in a device is from the compatible information is totally
sensible and reasonable.  Things like the I2C bus clock speed on the
other hand depend not on the individual device but on how it's been
integrated into the system so you'd end up saying things like
"nomadik-i2c-100khz-bus" in the compatible string which isn't great.  

If there's something like variation in the maximum speed that can be
supported by the different silicon the driver handles then working that
out from compatible and using it as the default is sensible but that's
not the same thing as providing customisation of the system bus
frequency to the user and if there's only one of those two that's
configurable it should be the system bus frequency.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: broonie@opensource.wolfsonmicro.com (Mark Brown)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 09/14] i2c: Add Device Tree support to the Nomadik I2C driver
Date: Fri, 15 Jun 2012 10:32:32 +0100	[thread overview]
Message-ID: <20120615093231.GB3898@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <4FDA341C.8010501@linaro.org>

On Thu, Jun 14, 2012 at 07:57:32PM +0100, Lee Jones wrote:
> On 14/06/12 19:36, Mark Brown wrote:

> >You're not understanding Linus' point.  The compatible string isn't
> >useful here because properties like the maximum clock rate of the bus
> >depend on the board design, not the silicon.  The controller may be
> >perfectly happy to run at a given rate but other devices on the bus or
> >the electrical engineering of the PCB itself may restrict this further.

> And you're not understanding mine. ;)

I think we are (or at least I do)...

> You can have multiple compatible strings for a single driver. Which
> one you reference from the Device Tree will dictate which group of
> settings are used, including variation of clock rates.

I'm certainly well aware of that.  The problem is that the compatible
strings should identify a particular IP or piece of silicon and be
unchanging properties of that hardware - for example, working out how
big a FIFO in a device is from the compatible information is totally
sensible and reasonable.  Things like the I2C bus clock speed on the
other hand depend not on the individual device but on how it's been
integrated into the system so you'd end up saying things like
"nomadik-i2c-100khz-bus" in the compatible string which isn't great.  

If there's something like variation in the maximum speed that can be
supported by the different silicon the driver handles then working that
out from compatible and using it as the default is sensible but that's
not the same thing as providing customisation of the system bus
frequency to the user and if there's only one of those two that's
configurable it should be the system bus frequency.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120615/017b558e/attachment.sig>

  reply	other threads:[~2012-06-15  9:32 UTC|newest]

Thread overview: 234+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-11 15:24 [PATCH 00/14] Final (large) batch of Snowball Device Tree Enablement Lee Jones
2012-06-11 15:24 ` Lee Jones
2012-06-11 15:24 ` [PATCH 01/14] rtc: ab8500-rtc: IRQF_ONESHOT when requesting a threaded IRQ Lee Jones
2012-06-11 15:24   ` Lee Jones
2012-06-11 15:31   ` Arnd Bergmann
2012-06-11 15:31     ` Arnd Bergmann
2012-06-11 15:37     ` Lee Jones
2012-06-11 15:37       ` Lee Jones
2012-06-11 15:51       ` Greg Kroah-Hartman
2012-06-11 15:51         ` Greg Kroah-Hartman
2012-06-11 16:01         ` Lee Jones
2012-06-11 16:01           ` Lee Jones
2012-06-11 20:54           ` [rtc-linux] " Linus Walleij
2012-06-11 20:54             ` Linus Walleij
2012-06-12  8:57     ` Lee Jones
2012-06-12  8:57       ` Lee Jones
2012-06-13 22:25       ` Andrew Morton
2012-06-13 22:25         ` Andrew Morton
2012-06-14  7:14         ` Lee Jones
2012-06-14  7:14           ` Lee Jones
2012-06-14  7:30           ` Andrew Morton
2012-06-14  7:30             ` Andrew Morton
2012-06-14  8:02             ` Lee Jones
2012-06-14  8:02               ` Lee Jones
2012-06-11 15:24 ` [PATCH 02/14] rtc: Ensure correct probing of the AB8500 RTC when Device Tree is enabled Lee Jones
2012-06-11 15:24   ` Lee Jones
2012-06-11 20:56   ` [rtc-linux] " Linus Walleij
2012-06-11 20:56     ` Linus Walleij
2012-07-02 15:23   ` Lee Jones
2012-07-02 15:23     ` Lee Jones
2012-06-11 15:24 ` [PATCH 03/14] mfd: ab8500-gpadc: Enable IRQF_ONESHOT when requesting a threaded IRQ Lee Jones
2012-06-11 15:24   ` Lee Jones
2012-06-11 20:54   ` Linus Walleij
2012-06-11 20:54     ` Linus Walleij
2012-06-12  8:58   ` Lee Jones
2012-06-12  8:58     ` Lee Jones
2012-06-11 15:24 ` [PATCH 04/14] mfd: Remove redundant Kconfig entry Lee Jones
2012-06-11 15:24   ` Lee Jones
2012-06-11 20:57   ` Linus Walleij
2012-06-11 20:57     ` Linus Walleij
2012-06-11 15:24 ` [PATCH 05/14] mfd: Enable DT probing of the DB8500 PRCMU Lee Jones
2012-06-11 15:24   ` Lee Jones
2012-06-11 20:58   ` Linus Walleij
2012-06-11 20:58     ` Linus Walleij
2012-06-29 14:47   ` Samuel Ortiz
2012-06-29 14:47     ` Samuel Ortiz
2012-06-11 15:24 ` [PATCH 06/14] mfd: Initialise the DB8500 PRCMU driver at core_initcall time Lee Jones
2012-06-11 15:24   ` Lee Jones
2012-06-11 21:01   ` Linus Walleij
2012-06-11 21:01     ` Linus Walleij
2012-06-12  8:11     ` Lee Jones
2012-06-12  8:11       ` Lee Jones
2012-06-12  8:35     ` Lee Jones
2012-06-12  8:35       ` Lee Jones
2012-06-12 12:59       ` Arnd Bergmann
2012-06-12 12:59         ` Arnd Bergmann
2012-06-12 13:23         ` Lee Jones
2012-06-12 13:23           ` Lee Jones
2012-06-12 14:49           ` Arnd Bergmann
2012-06-12 14:49             ` Arnd Bergmann
2012-06-12 15:39             ` Lee Jones
2012-06-12 15:39               ` Lee Jones
2012-06-18  8:32             ` Lee Jones
2012-06-18  8:32               ` Lee Jones
2012-06-29 14:48   ` Samuel Ortiz
2012-06-29 14:48     ` Samuel Ortiz
2012-06-11 15:25 ` [PATCH 07/14] mfd: Initialise the AB8500 " Lee Jones
2012-06-11 15:25   ` Lee Jones
2012-06-11 21:03   ` Linus Walleij
2012-06-11 21:03     ` Linus Walleij
2012-06-12  8:13     ` Lee Jones
2012-06-12  8:13       ` Lee Jones
2012-06-12  8:37     ` Lee Jones
2012-06-12  8:37       ` Lee Jones
2012-06-29 14:51   ` Samuel Ortiz
2012-06-29 14:51     ` Samuel Ortiz
2012-06-11 15:25 ` [PATCH 08/14] mfd: Add IRQ domain support for the AB8500 Lee Jones
2012-06-11 15:25   ` Lee Jones
2012-06-11 21:33   ` Linus Walleij
2012-06-11 21:33     ` Linus Walleij
2012-06-12  8:01     ` Lee Jones
2012-06-12  8:01       ` Lee Jones
2012-06-14 18:32   ` Mark Brown
2012-06-14 18:32     ` Mark Brown
2012-06-15 12:49     ` Lee Jones
2012-06-15 12:49       ` Lee Jones
2012-06-18  9:02     ` Lee Jones
2012-06-18  9:02       ` Lee Jones
2012-06-18  9:32       ` Mark Brown
2012-06-18  9:32         ` Mark Brown
2012-06-18 10:20       ` Russell King - ARM Linux
2012-06-18 10:20         ` Russell King - ARM Linux
2012-06-18 18:56         ` Mark Brown
2012-06-18 18:56           ` Mark Brown
2012-06-20 13:00           ` Lee Jones
2012-06-20 13:00             ` Lee Jones
2012-06-21  7:37             ` Linus Walleij
2012-06-21  7:37               ` Linus Walleij
2012-06-26  9:04               ` Lee Jones
2012-06-26  9:04                 ` Lee Jones
2012-06-20  9:12         ` Linus Walleij
2012-06-20  9:12           ` Linus Walleij
2012-06-11 15:25 ` [PATCH 09/14] i2c: Add Device Tree support to the Nomadik I2C driver Lee Jones
2012-06-11 15:25   ` Lee Jones
2012-06-11 19:05   ` Wolfram Sang
2012-06-11 19:05     ` Wolfram Sang
2012-06-12  7:23     ` Lee Jones
2012-06-12  7:23       ` Lee Jones
2012-06-12  8:52     ` Lee Jones
2012-06-12  8:52       ` Lee Jones
2012-06-12  8:52       ` Lee Jones
2012-06-12  8:57       ` Wolfram Sang
2012-06-12  8:57         ` Wolfram Sang
2012-06-12  8:57         ` Wolfram Sang
2012-06-11 20:37   ` Linus Walleij
2012-06-11 20:37     ` Linus Walleij
2012-06-11 20:37     ` Linus Walleij
2012-06-12  7:34     ` Lee Jones
2012-06-12  7:34       ` Lee Jones
2012-06-12  7:34       ` Lee Jones
2012-06-13  5:40       ` Linus Walleij
2012-06-13  5:40         ` Linus Walleij
2012-06-13  5:40         ` Linus Walleij
2012-06-13  7:01         ` Lee Jones
2012-06-13  7:01           ` Lee Jones
2012-06-13  7:01           ` Lee Jones
2012-06-13  8:12           ` Linus Walleij
2012-06-13  8:12             ` Linus Walleij
2012-06-13 12:28             ` Lee Jones
2012-06-13 12:28               ` Lee Jones
2012-06-13 12:28               ` Lee Jones
2012-06-14 18:36               ` Mark Brown
2012-06-14 18:36                 ` Mark Brown
2012-06-14 18:36                 ` Mark Brown
2012-06-14 18:46                 ` Mark Brown
2012-06-14 18:46                   ` Mark Brown
2012-06-14 18:46                   ` Mark Brown
2012-06-14 18:59                   ` Lee Jones
2012-06-14 18:59                     ` Lee Jones
2012-06-14 18:57                 ` Lee Jones
2012-06-14 18:57                   ` Lee Jones
2012-06-14 18:57                   ` Lee Jones
2012-06-15  9:32                   ` Mark Brown [this message]
2012-06-15  9:32                     ` Mark Brown
2012-06-15  9:32                     ` Mark Brown
2012-06-15 10:00                     ` Lee Jones
2012-06-15 10:00                       ` Lee Jones
2012-06-15 10:32                   ` Russell King - ARM Linux
2012-06-15 10:32                     ` Russell King - ARM Linux
2012-06-15 11:43                     ` Lee Jones
2012-06-15 11:43                       ` Lee Jones
2012-06-15 11:43                       ` Lee Jones
2012-06-13 16:07     ` [PATCH 1/3] " Lee Jones
2012-06-13 16:07       ` Lee Jones
2012-06-13 16:07       ` Lee Jones
2012-06-14 17:12       ` Linus Walleij
2012-06-14 17:12         ` Linus Walleij
2012-06-14 17:12         ` Linus Walleij
2012-06-15  9:02         ` Lee Jones
2012-06-15  9:02           ` Lee Jones
2012-06-15  9:02           ` Lee Jones
     [not found]           ` <CAE2-_9rRYxjU9QQgtBv9ReMY5x+oRiJG1cDQahYHanjDrwVUYA@mail.gmail.com>
2012-06-15 11:50             ` Fwd: " Srinidhi Kasagar
2012-06-15 11:50               ` Srinidhi Kasagar
2012-06-15 11:50               ` Srinidhi Kasagar
2012-06-15 12:45               ` Lee Jones
2012-06-15 12:45                 ` Lee Jones
2012-06-15 12:45                 ` Lee Jones
2012-06-15 13:05                 ` Srinidhi Kasagar
2012-06-15 13:05                   ` Srinidhi Kasagar
2012-06-15 13:05                   ` Srinidhi Kasagar
2012-06-15 13:18                   ` Lee Jones
2012-06-15 13:18                     ` Lee Jones
2012-06-15 13:18                     ` Lee Jones
2012-06-15 13:37                     ` Srinidhi Kasagar
2012-06-15 13:37                       ` Srinidhi Kasagar
2012-06-15 13:37                       ` Srinidhi Kasagar
2012-06-15 13:58                       ` Lee Jones
2012-06-15 13:58                         ` Lee Jones
2012-06-15 13:58                         ` Lee Jones
2012-06-17 17:43                     ` Linus Walleij
2012-06-17 17:43                       ` Linus Walleij
2012-06-17 17:43                       ` Linus Walleij
2012-06-18  7:18                       ` Lee Jones
2012-06-18  7:18                         ` Lee Jones
2012-06-18  7:18                         ` Lee Jones
2012-06-18  7:58                         ` Srinidhi Kasagar
2012-06-18  7:58                           ` Srinidhi Kasagar
2012-06-18  7:58                           ` Srinidhi Kasagar
2012-06-18  8:41                           ` Lee Jones
2012-06-18  8:41                             ` Lee Jones
2012-06-18  8:41                             ` Lee Jones
2012-06-13 16:08     ` [PATCH 2/3] ARM: ux500: Add i2c configurations to the Device Tree for DB8500 based devices Lee Jones
2012-06-13 16:08       ` Lee Jones
2012-06-14 17:13       ` Linus Walleij
2012-06-14 17:13         ` Linus Walleij
2012-06-14 17:13         ` Linus Walleij
2012-06-18 11:29       ` Linus Walleij
2012-06-18 11:29         ` Linus Walleij
2012-06-18 11:29         ` Linus Walleij
2012-06-18 11:37         ` Lee Jones
2012-06-18 11:37           ` Lee Jones
2012-06-18 11:37           ` Lee Jones
2012-06-13 16:08     ` [PATCH 3/3] Documentation: Device Tree binding information for i2c-nomadik driver Lee Jones
2012-06-13 16:08       ` Lee Jones
2012-06-13 16:08       ` Lee Jones
2012-06-14 17:13       ` Linus Walleij
2012-06-14 17:13         ` Linus Walleij
2012-06-14 17:13         ` Linus Walleij
2012-06-11 15:25 ` [PATCH 10/14] Input: Add Device Tree support to the ab8500-ponkey driver Lee Jones
2012-06-11 15:25   ` Lee Jones
2012-06-11 15:25 ` [PATCH 11/14] usb: otg: Enable probing of the ab8500 during a Device Tree boot Lee Jones
2012-06-11 15:25   ` Lee Jones
2012-06-11 21:39   ` Linus Walleij
2012-06-11 21:39     ` Linus Walleij
2012-07-02 15:24   ` Lee Jones
2012-07-02 15:24     ` Lee Jones
2012-06-11 15:25 ` [PATCH 12/14] ARM: ux500: Correctly reference IRQs supplied by the AB8500 from Device Tree Lee Jones
2012-06-11 15:25   ` Lee Jones
2012-06-11 21:41   ` Linus Walleij
2012-06-11 21:41     ` Linus Walleij
2012-06-11 15:25 ` [PATCH 13/14] ARM: ux500: Enable the AB8500 RTC for all DT:ed DB8500 based devices Lee Jones
2012-06-11 15:25   ` Lee Jones
2012-06-11 21:42   ` Linus Walleij
2012-06-11 21:42     ` Linus Walleij
2012-06-11 15:25 ` [PATCH 14/14] ARM: ux500: Move rtc-pl031 registration to Device Tree when enabled Lee Jones
2012-06-11 15:25   ` Lee Jones
2012-06-11 21:43   ` Linus Walleij
2012-06-11 21:43     ` Linus Walleij
2012-06-12  8:03     ` Lee Jones
2012-06-12  8:03       ` Lee Jones
2012-06-12 14:32       ` Linus Walleij
2012-06-12 14:32         ` Linus Walleij
2012-06-12 14:44         ` Lee Jones
2012-06-12 14:44           ` Lee Jones

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=20120615093231.GB3898@opensource.wolfsonmicro.com \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=arnd@arndb.de \
    --cc=grant.likely@secretlab.ca \
    --cc=lee.jones@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linus.walleij@stericsson.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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.