Re: [PATCH v2 8/8] i2c-mux: relax locking of the top i2c adapter during i2c controlled muxing

[Rob Herring]

On Tue, Jan 05, 2016 at 04:57:18PM +0100, Peter Rosin wrote:
> From: Peter Rosin 
> 
> With a i2c topology like the following
> 
>GPIO ---|  -- BAT1
> |  v /
>I2C  -+--+ MUX
>  |   \
>EEPROM -- BAT2

Yuck. One would think you would just use an I2C controlled mux in this 
case...
 
> there is a locking problem with the GPIO controller since it is a client
> on the same i2c bus that it muxes. Transfers to the mux clients (e.g. BAT1)
> will lock the whole i2c bus prior to attempting to switch the mux to the
> correct i2c segment. In the above case, the GPIO device is an I/O expander
> with an i2c interface, and since the GPIO subsystem knows nothing (and
> rightfully so) about the lockless needs of the i2c mux code, this results
> in a deadlock when the GPIO driver issues i2c transfers to modify the
> mux.
> 
> So, observing that while it is needed to have the i2c bus locked during the
> actual MUX update in order to avoid random garbage on the slave side, it
> is not strictly a must to have it locked over the whole sequence of a full
> select-transfer-deselect mux client operation. The mux itself needs to be
> locked, so transfers to clients behind the mux are serialized, and the mux
> needs to be stable during all i2c traffic (otherwise individual mux slave
> segments might see garbage, or worse).
> 
> Add devive tree properties (bool named i2c-controlled) to i2c-mux-gpio and
> i2c-mux-pinctrl that asserts that the the gpio/pinctrl is controlled via
> the same i2c bus that it muxes.

Can't you determine this condition by checking the mux parent and gpio 
parent are the same i2c controller?

Alternatively, can't you just always do the locking like i2c-controlled 
is set when a mux is involved? What is the harm in doing that if the 
GPIO is controlled somewhere else?

I would prefer to see a solution not requiring DT updates to fix and 
this change seems like it is working around kernel issues.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver

[Rob Herring]

On Wed, Jan 6, 2016 at 7:51 AM, Wolfram Sang  wrote:
> From: Wolfram Sang 
>
> This driver allows an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be
> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.
>
> Signed-off-by: Wolfram Sang 

[...]

> +static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, 
> u32 new_chan)
> +{
> +   struct i2c_adapter *adap;
> +   struct pinctrl *p;
> +   int ret;
> +
> +   mutex_lock(_mutex);
> +   ret = of_changeset_apply(>chan[new_chan].chgset);
> +   mutex_unlock(_mutex);

Looks like you need this patch[1] rather than exposing of_mutex.

Rob

[1] https://patchwork.ozlabs.org/patch/539938/
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] bindings: i2c: Add clock entries for i2c-xiic

[Rob Herring]

On Mon, Jan 04, 2016 at 01:20:34PM +0530, Shubhrajyoti Datta wrote:
> Add clock description for i2c-xiic
> 
> Signed-off-by: Shubhrajyoti Datta <shubh...@xilinx.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-xiic.txt |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Acked-by: Rob Herring <r...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] DT: i2c: Add Freescale MPL3115 to trivial devices list

[Rob Herring]

On Wed, Dec 23, 2015 at 01:59:07PM -0500, Akshay Bhat wrote:
> This adds devicetree documentation for the bindings of mpl3115 driver.
> 
> Signed-off-by: Akshay Bhat <akshay.b...@timesys.com>
> ---
>  Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <r...@kernel.org>

Who did you want to apply this? Send patches TO that person which I 
would expect to be Wolfram or me.

Rob

> 
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt 
> b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index c50cf13..f3f4bc2 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -54,6 +54,7 @@ epson,rx8581I2C-BUS INTERFACE REAL TIME 
> CLOCK MODULE
>  fsl,mag3110  MAG3110: Xtrinsic High Accuracy, 3D Magnetometer
>  fsl,mc13892  MC13892: Power Management Integrated Circuit (PMIC) for 
> i.MX35/51
>  fsl,mma8450  MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic 
> Accelerometer
> +fsl,mpl3115  MPL3115: Absolute Digital Pressure Sensor
>  fsl,mpr121   MPR121: Proximity Capacitive Touch Sensor Controller
>  fsl,sgtl5000 SGTL5000: Ultra Low-Power Audio Codec
>  gmt,g751 G751: Digital Temperature Sensor and Thermal Watchdog 
> with Two-Wire Interface
> -- 
> 2.6.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/3] i2c: document binding for multi-master case

[Rob Herring]

On Wed, Dec 16, 2015 at 07:44:18PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+rene...@sang-engineering.com>
> 
> We need this binding because some I2C master drivers will need to adapt
> their PM settings for the arbitration circuitry. I skipped the "i2c-"
> prefix because I can imagine to be useful outside of i2c.
> 
> Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
> Cc: devicet...@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/i2c/i2c.txt | 5 +++++
>  1 file changed, 5 insertions(+)

Acked-by: Rob Herring <r...@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] i2c: document generic DT bindings for timing parameters

[Rob Herring]

On Thu, Dec 03, 2015 at 04:51:31PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+rene...@sang-engineering.com>
> 
> Also, sort the properties alphabetically and make indentation
> consistent. Wording largely taken from i2c-rk3x.txt, thanks guys!
> 
> Only "i2c-scl-internal-delay-ns" is new, the rest is used by two drivers
> already and was documented in their driver binding documentation.
> 
> Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
> Cc: devicet...@vger.kernel.org

Acked-by: Rob Herring <r...@kernel.org>

> ---
>  Documentation/devicetree/bindings/i2c/i2c.txt | 31 
> ++-
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt 
> b/Documentation/devicetree/bindings/i2c/i2c.txt
> index 8a99150ac3a7fd..a00219f5ee0733 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -29,12 +29,33 @@ Optional properties
>  These properties may not be supported by all drivers. However, if a driver
>  wants to support one of the below features, it should adapt the bindings 
> below.
>  
> -- clock-frequency- frequency of bus clock in Hz.
> -- wakeup-source  - device can be used as a wakeup source.
> +- clock-frequency
> + frequency of bus clock in Hz.
>  
> -- interrupts - interrupts used by the device.
> -- interrupt-names- "irq" and "wakeup" names are recognized by I2C core,
> -   other names are left to individual drivers.
> +- i2c-scl-falling-time-ns
> + Number of nanoseconds the SCL signal takes to fall; t(f) in the I2C
> + specification.
> +
> +- i2c-scl-internal-delay-ns
> + Number of nanoseconds the IP core additionally needs to setup SCL.
> +
> +- i2c-scl-rising-time-ns
> + Number of nanoseconds the SCL signal takes to rise; t(r) in the I2C
> + specification.
> +
> +- i2c-sda-falling-time-ns
> + Number of nanoseconds the SDA signal takes to fall; t(f) in the I2C
> + specification.
> +
> +- interrupts
> + interrupts used by the device.
> +
> +- interrupt-names
> + "irq" and "wakeup" names are recognized by I2C core, other names are
> + left to individual drivers.
> +
> +- wakeup-source
> + device can be used as a wakeup source.
>  
>  Binding may contain optional "interrupts" property, describing interrupts
>  used by the device. I2C core will assign "irq" interrupt (or the very first
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] i2c: at91: update bindings documention

[Rob Herring]

On Wed, Dec 02, 2015 at 11:39:05AM +0100, Ludovic Desroches wrote:
> The i2c-sda-hold-time-ns property is supported from atmel,sama5d4-i2c.
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>

Acked-by: Rob Herring <r...@kernel.org>

> ---
>  Documentation/devicetree/bindings/i2c/i2c-at91.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt 
> b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> index 6e81dc1..67c6f2e 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> @@ -3,7 +3,7 @@ I2C for Atmel platforms
>  Required properties :
>  - compatible : Must be "atmel,at91rm9200-i2c", "atmel,at91sam9261-i2c",
>   "atmel,at91sam9260-i2c", "atmel,at91sam9g20-i2c", 
> "atmel,at91sam9g10-i2c",
> - "atmel,at91sam9x5-i2c" or "atmel,sama5d2-i2c"
> + "atmel,at91sam9x5-i2c", "atmel,sama5d4-i2c" or "atmel,sama5d2-i2c"
>  - reg: physical base address of the controller and length of memory mapped
>   region.
>  - interrupts: interrupt number to the cpu.
> @@ -17,6 +17,7 @@ Optional properties:
>  - dma-names: should contain "tx" and "rx".
>  - atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for 
> FIFO
>capable I2C controllers.
> +- i2c-sda-hold-time-ns: TWD hold time, only available from 
> "atmel,sama5d4-i2c".
>  - Child nodes conforming to i2c bus binding
>  
>  Examples :
> @@ -52,6 +53,7 @@ i2c0: i2c@f8034600 {
>   #size-cells = <0>;
>   clocks = <>;
>   atmel,fifo-size = <16>;
> + i2c-sda-hold-time-ns = <336>;
>  
>   wm8731: wm8731@1a {
>   compatible = "wm8731";
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] i2c: at91: add DT property "atmel,twd-hold-cycles" to binding

[Rob Herring]

On Tue, Nov 24, 2015 at 02:47:41PM +0100, Ludovic Desroches wrote:
> From: Wenyou Yang <wenyou.y...@atmel.com>
> 
> Add a DT property "atmel,twd-hold-cycles" to specify the HOLD
> filed of TWIHS_CWGR register to increase the TWD hold time.
> 
> Signed-off-by: Wenyou Yang <wenyou.y...@atmel.com>
> Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>

Acked-by: Rob Herring <r...@kernel.org>

> ---
>  Documentation/devicetree/bindings/i2c/i2c-at91.txt | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt 
> b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> index 6e81dc1..c81a0cb 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> @@ -17,6 +17,9 @@ Optional properties:
>  - dma-names: should contain "tx" and "rx".
>  - atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for 
> FIFO
>capable I2C controllers.
> +- atmel,twd-hold-cycles: number of cycles for TWD hold time whose value is
> +  determinated by (atmel,twd-hold-cycles + 3) x t_peripheral_clock,
> +  maximum value is 0x1f.
>  - Child nodes conforming to i2c bus binding
>  
>  Examples :
> @@ -29,6 +32,7 @@ i2c0: i2c@fff84000 {
>   #size-cells = <0>;
>   clocks = <_clk>;
>   clock-frequency = <40>;
> + atmel,twd-hold-cycles = <2>;
>  
>   24c512@50 {
>   compatible = "24c512";
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] devicetree: i2c: add binding for Sigma Designs SMP8642 I2C master

[Rob Herring]

On Sat, Oct 31, 2015 at 12:35 PM, Mans Rullgard <m...@mansr.com> wrote:
> This adds a binding for the Sigma Designs SMP8642 built-in I2C master
> controller.
>
> Signed-off-by: Mans Rullgard <m...@mansr.com>

Acked-by: Rob Herring <r...@kernel.org>

> ---
>  .../devicetree/bindings/i2c/sigma,smp8642-i2c.txt  | 24 
> ++
>  1 file changed, 24 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/i2c/sigma,smp8642-i2c.txt
>
> diff --git a/Documentation/devicetree/bindings/i2c/sigma,smp8642-i2c.txt 
> b/Documentation/devicetree/bindings/i2c/sigma,smp8642-i2c.txt
> new file mode 100644
> index 000..117fc8b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/sigma,smp8642-i2c.txt
> @@ -0,0 +1,24 @@
> +Sigma Designs SMP8642 I2C master
> +
> +Required properties:
> +- compatible: should be "sigma,smp8642-i2c"
> +- reg: physical address space of the device
> +- interrupts: the interrupt of the device
> +- clocks: phandle of the clock for the device
> +- #address-cells: should be <1>
> +- #size-cells: should be <0>
> +
> +Optional properties:
> +- clock-frequency: frequency of bus clock in Hz, default 100kHz
> +
> +Example:
> +
> +i2c@10480 {
> +   compatible = "sigma,smp8642-i2c";
> +   reg = <0x10480 0x2c>;
> +   interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;
> +   clocks = <_clk>;
> +   clock-frequency = <10>;
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +};
> --
> 2.6.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Fri, Oct 23, 2015 at 10:45 AM, Tim Bird  wrote:
> On 10/22/2015 11:53 AM, Frank Rowand wrote:
>> On 10/22/2015 7:44 AM, Greg Kroah-Hartman wrote:
>>> 
>>>
>>> On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote:
 But that's moot currently because Greg believes that the time spent
 probing devices at boot time could be reduced enough so that the order
 in which devices are probed becomes irrelevant. IME that would have to
 be under 200ms so that the user doesn't notice and that's unicorn-far
 from any bootlog I have ever seen.
>>>
>>> But as no one has actually produced a bootlog, how do you know that?
>>> Where exactly is your time being spent?  What driver is causing long
>>> delays?  Why is the long-delay-drivers not being done in their own
>>> thread?  And most importantly, why are you ignoring the work that people
>>> did back in 2008 to solve the issue on other hardware platforms?
>>>
 Given that downstreams are already carrying as many hacks as they
 could think of to speed total boot up, I think this is effectively
 telling them to go away.
>>>
>>> No I'm not, I'm asking for real data, not hand-wavy-this-is-going-to
>>> solve-the-random-issue-i'm-having type patch by putting random calls in
>>> semi-random subsystems all over the kernel.
>>>
>>> And when I ask for real data, you respond with the fact that you aren't
>>> trying to speed up boot time here at all, so what am I supposed to think
>>
>> I also had the understanding that this patch series was about improving
>> boot time.  But I was kindly corrected that the behavior change was
>> getting the panel displaying stuff at an earlier point in the boot sequence,
>> _not_ completing the entire boot faster.
>>
>> The claim for the current series, in patch 0 in v7 is:
>>
>>With this series I get the kernel to output to the panel in 0.5s,
>>instead of 2.8s.
>
> It's very common to want to get the display up before the
> rest of the system.  So wanting to accelerate one part of the boot
> at the expense to the rest of the system is a valid use case.
> Deferred initcalls, which is out of tree primarily because it requires
> the type of manual tweaking that Tomeu describes, specifically
> addressed this issue.

Agreed and other folks will want other things up first. But it seems
we are getting lucky with link order with the speed ups in this case.
We need a way to specify priority of probing devices. If we have that
piece, then all this plumbing can be used. A simple solution would be
looking at stdout-path to get the console device to probe. That would
be trivial to add on top of this. That may work for the display too,
but you may not want the console on the display. That wouldn't work
for CAN bus either, but then I'm not sure there is a generic solution
for its requirements (respond within 50ms IIRC).

>> Just to get side-tracked, one other approach at ordering to reduce
>> deferrals reported a modest boot time reduction for four boards and a
>> very slight boot time increase for one other board.) The report of boot
>> times with that approach was in:
>>
>>   http://article.gmane.org/gmane.linux.drivers.devicetree/133010
>>
>> from Alexander Holler.
>>
>> I have not searched further to see if there is more data of boot time
>> reductions from any of the other attempts to change driver binding
>> order to move dependencies before use of a resource.  But whether
>> there is a performance improvement or not, there continues to be
>> a stream of developers creatively impacting the binding order for
>> their specific driver(s) or board.  So it seems that maybe there
>> is an underlying problem, or we don't have adequate documentation
>> explaining how to avoid a need to order bindings, or the
>> documentation exists and is not being read.
>
> Well, I have probe order problems unrelated to boot time, that
> I solved by resorting to putting stuff into modules and loading
> them post-boot.  So I'd be interested in easy solutions to managing
> boot order in mainline.

I take it that this series doesn't help those problems?

>> I have been defaulting to the position that has been asserted by
>> the device tree maintainters, that probe deferrals work just fine
>> for at least the majority of cases (and is the message I have been
>> sharing in my conference presentations about device tree).  But I
>> suspect that there is at least a small minority of cases that are not
>> well served by probe deferral.  (Not to be read as an endorsement of
>> this specific patch series, just a generic observation.)
>
> I've been worried about DT overhead adding to boot time for a while.

Always beating up DT... ;) Yes, I'm sure there is some overhead, but
looking at bootgraph there's much longer items not related to DT (USB,
MMC and anything over I2C seem to be typical). With DT we lost most
control of the order, and at the same time we added a load of new
subsystems that are 

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Wed, Oct 21, 2015 at 1:18 PM, Frank Rowand  wrote:
> On 10/21/2015 9:27 AM, Mark Brown wrote:
>> On Wed, Oct 21, 2015 at 08:59:51AM -0700, Frank Rowand wrote:
>>> On 10/19/2015 5:34 AM, Tomeu Vizoso wrote:
>>
 To be clear, I was saying that this series should NOT affect total
 boot times much.
>>
>>> I'm confused.  If I understood correctly, improving boot time was
>>> the key justification for accepting this patch set.  For example,
>>> from "[PATCH v7 0/20] On-demand device probing":
>>>
>>>I have a problem with the panel on my Tegra Chromebook taking longer
>>>than expected to be ready during boot (Stéphane Marchesin reported what
>>>is basically the same issue in [0]), and have looked into ordered
>>>probing as a better way of solving this than moving nodes around in the
>>>DT or playing with initcall levels and linking order.
>>>
>>>...
>>>
>>>With this series I get the kernel to output to the panel in 0.5s,
>>>instead of 2.8s.
>>
>> Overall boot time and time to get some individual built in component up
>> and running aren't the same thing - what this'll do is get things up
>> more in the link order of the leaf consumers rather than deferring those
>> leaf consumers when their dependencies aren't ready yet.
>
> Thanks!  I read too much into what was being improved.
>
> So this patch series, which on other merits may be a good idea, is as
> a by product solving a specific ordering issue, moving successful panel
> initialization to an earlier point in the boot sequence, if I now
> understand more correctly.
>
> In that context, this seems like yet another ad hoc way of causing the
> probe order to change in a way to solves one specific issue?  Could
> it just as likely move the boot order of some other driver on some
> other board later, to the detriment of somebody else?

Time to display on is important for many products. Having the console
up as early as possible is another case. CAN bus is another. This is a
real problem that is not just bad drivers.

I don't think it is completely ad hoc. Given all devices are
registered after drivers, drivers will still probe first in initcall
level order and then link order AFAIK. We may not take (more) initcall
level tweak hacks, but that is a much more simple change for
downstream. Don't get me wrong, I'd really like to see a way to
control order independent of initcall level.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Tue, Oct 20, 2015 at 2:56 AM, Rafael J. Wysocki <r...@rjwysocki.net> wrote:
> On Monday, October 19, 2015 05:58:40 PM Rob Herring wrote:
>> On Mon, Oct 19, 2015 at 4:40 PM, Rafael J. Wysocki <r...@rjwysocki.net> 
>> wrote:
>> > On Monday, October 19, 2015 10:58:25 AM Rob Herring wrote:
>> >> On Mon, Oct 19, 2015 at 10:29 AM, David Woodhouse <dw...@infradead.org> 
>> >> wrote:
>> >> > On Mon, 2015-10-19 at 15:50 +0100, Mark Brown wrote:
>> >> >> > But the point I'm making is that we are working towards *fixing* 
>> >> >> > that,
>> >> >> > and *not* using DT-specific code in places where we should be using 
>> >> >> > the
>> >> >> > generic APIs.
>> >> >>
>> >> >> What is the plan for fixing things here?  It's not obvious (at least to
>> >> >> me) that we don't want to have the subsystems having knowledge of how
>> >> >> they are bound to a specific firmware which is what you seem to imply
>> >> >> here.
>> >> >
>> >> > I don't know that there *is* a coherent plan here to address it all.
>> >> >
>> >> > Certainly, we *will* need subsystems to have firmware-specific
>> >> > knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
>> >> > describe GPIO, and properties which reference GPIO pins are intended to
>> >> > work through that — while in DT, properties which reference GPIO pins
>> >> > will have different contents. They'll be compatible at the driver
>> >> > level, in the sense that there's a call to get a given GPIO given the
>> >> > property name, but the subsystems *will* be doing different things
>> >> > behind the scenes.
>> >> >
>> >> > My plan, such as it is, is to go through the leaf-node drivers which
>> >> > almost definitely *should* be firmware-agnostic, and convert those. And
>> >> > then take stock of what we have left, and work out what, if anything,
>> >> > still needs to be done.
>> >>
>> >> Many cases are already agnostic in the drivers in terms of the *_get()
>> >> functions. Some are DT specific, but probably because those subsystems
>> >> are new and DT only. In any case, I don't think these 1 line changes
>> >> do anything to make doing conversions here harder.
>> >>
>> >> >> It seems like we're going to have to refactor these bits of code when
>> >> >> they get generalised anyway so I'm not sure that the additional cost
>> >> >> here is that big.
>> >> >
>> >> > That's an acceptable answer — "we're adding legacy code here but we
>> >> > know it's going to be refactored anyway". If that's true, all it takes
>> >> > is a note in the commit comment to that effect. That's different from
>> >> > having not thought about it :)
>> >>
>> >> Considering at one point we did create a fwnode based API, we did
>> >> think about it. Plus there was little input from ACPI folks as to
>> >> whether the change was even useful for ACPI case.
>> >
>> > Well, sorry, but who was asking whom, specifically?
>>
>> You and linux-acpi have been copied on v2 and later of the entire
>> series I think.
>
> Yes, but it wasn't like a direct request, say "We need your input, so can you
> please have a look and BTW we want this in 4.4, so please do it ASAP".  In
> which case I'd prioritize that before other things I needed to take care of.

Fair enough. Can you please review and comment on v7 of the series? We
can discuss at KS as well.

>> > The underlying problem is present in ACPI too and we don't really have a 
>> > good
>> > solution for it.  We might benefit from a common one if it existed.
>>
>> The problem for DT is we don't generically know what are the
>> dependencies at a core level. We could know some or most dependencies
>> if phandles (links to other nodes) were typed, but they are not. If
>> the core had this information, we could simply control the device
>> creation to order probing. Instead, this information is encoded into
>> the bindings and binding parsing resides in the subsystems. That
>> parsing happens during probe of the client side and is done by the
>> subsystems (for common bindings). Since we already do the parsing at
>> this point, it is a convenient place to t

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Mon, Oct 19, 2015 at 4:44 AM, David Woodhouse  wrote:
> On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote:
>> On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote:
>> > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote:
>> > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote:
>>
>> > > > I can't see adding calls like this all over the tree just to solve a
>> > > > bus-specific problem, you are adding of_* calls where they aren't
>> > > > needed, or wanted, at all.
>>
>> > > This isn't bus specific, I'm not sure what makes you say that?
>>
>> > You are making it bus-specific by putting these calls all over the tree
>> > in different bus subsystems semi-randomly for all I can determine.
>>
>> Do you mean firmware rather than bus here?  I think that's the confusion
>> I have...
>
> Certainly, if it literally is adding of_* calls then that would seem to
> be gratuitously firmware-specific. Nothing should be using those these
> days; any new code should be using the generic device property APIs
> (except in special cases).

See version 2 of the series[1] which did that. It became obvious that
was pointless because the call paths ended up looking like this:

Generic subsystem code -> DT look-up code -> fwnode_probe_device ->
of_probe_device

Rob

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/361137.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Mon, Oct 19, 2015 at 10:29 AM, David Woodhouse  wrote:
> On Mon, 2015-10-19 at 15:50 +0100, Mark Brown wrote:
>> > But the point I'm making is that we are working towards *fixing* that,
>> > and *not* using DT-specific code in places where we should be using the
>> > generic APIs.
>>
>> What is the plan for fixing things here?  It's not obvious (at least to
>> me) that we don't want to have the subsystems having knowledge of how
>> they are bound to a specific firmware which is what you seem to imply
>> here.
>
> I don't know that there *is* a coherent plan here to address it all.
>
> Certainly, we *will* need subsystems to have firmware-specific
> knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
> describe GPIO, and properties which reference GPIO pins are intended to
> work through that — while in DT, properties which reference GPIO pins
> will have different contents. They'll be compatible at the driver
> level, in the sense that there's a call to get a given GPIO given the
> property name, but the subsystems *will* be doing different things
> behind the scenes.
>
> My plan, such as it is, is to go through the leaf-node drivers which
> almost definitely *should* be firmware-agnostic, and convert those. And
> then take stock of what we have left, and work out what, if anything,
> still needs to be done.

Many cases are already agnostic in the drivers in terms of the *_get()
functions. Some are DT specific, but probably because those subsystems
are new and DT only. In any case, I don't think these 1 line changes
do anything to make doing conversions here harder.

>> It seems like we're going to have to refactor these bits of code when
>> they get generalised anyway so I'm not sure that the additional cost
>> here is that big.
>
> That's an acceptable answer — "we're adding legacy code here but we
> know it's going to be refactored anyway". If that's true, all it takes
> is a note in the commit comment to that effect. That's different from
> having not thought about it :)

Considering at one point we did create a fwnode based API, we did
think about it. Plus there was little input from ACPI folks as to
whether the change was even useful for ACPI case. In any case, we're
talking about adding 1 line.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Mon, Oct 19, 2015 at 4:40 PM, Rafael J. Wysocki <r...@rjwysocki.net> wrote:
> On Monday, October 19, 2015 10:58:25 AM Rob Herring wrote:
>> On Mon, Oct 19, 2015 at 10:29 AM, David Woodhouse <dw...@infradead.org> 
>> wrote:
>> > On Mon, 2015-10-19 at 15:50 +0100, Mark Brown wrote:
>> >> > But the point I'm making is that we are working towards *fixing* that,
>> >> > and *not* using DT-specific code in places where we should be using the
>> >> > generic APIs.
>> >>
>> >> What is the plan for fixing things here?  It's not obvious (at least to
>> >> me) that we don't want to have the subsystems having knowledge of how
>> >> they are bound to a specific firmware which is what you seem to imply
>> >> here.
>> >
>> > I don't know that there *is* a coherent plan here to address it all.
>> >
>> > Certainly, we *will* need subsystems to have firmware-specific
>> > knowledge in some cases. Take GPIO as an example; ACPI *has* a way to
>> > describe GPIO, and properties which reference GPIO pins are intended to
>> > work through that — while in DT, properties which reference GPIO pins
>> > will have different contents. They'll be compatible at the driver
>> > level, in the sense that there's a call to get a given GPIO given the
>> > property name, but the subsystems *will* be doing different things
>> > behind the scenes.
>> >
>> > My plan, such as it is, is to go through the leaf-node drivers which
>> > almost definitely *should* be firmware-agnostic, and convert those. And
>> > then take stock of what we have left, and work out what, if anything,
>> > still needs to be done.
>>
>> Many cases are already agnostic in the drivers in terms of the *_get()
>> functions. Some are DT specific, but probably because those subsystems
>> are new and DT only. In any case, I don't think these 1 line changes
>> do anything to make doing conversions here harder.
>>
>> >> It seems like we're going to have to refactor these bits of code when
>> >> they get generalised anyway so I'm not sure that the additional cost
>> >> here is that big.
>> >
>> > That's an acceptable answer — "we're adding legacy code here but we
>> > know it's going to be refactored anyway". If that's true, all it takes
>> > is a note in the commit comment to that effect. That's different from
>> > having not thought about it :)
>>
>> Considering at one point we did create a fwnode based API, we did
>> think about it. Plus there was little input from ACPI folks as to
>> whether the change was even useful for ACPI case.
>
> Well, sorry, but who was asking whom, specifically?

You and linux-acpi have been copied on v2 and later of the entire
series I think.

> The underlying problem is present in ACPI too and we don't really have a good
> solution for it.  We might benefit from a common one if it existed.

The problem for DT is we don't generically know what are the
dependencies at a core level. We could know some or most dependencies
if phandles (links to other nodes) were typed, but they are not. If
the core had this information, we could simply control the device
creation to order probing. Instead, this information is encoded into
the bindings and binding parsing resides in the subsystems. That
parsing happens during probe of the client side and is done by the
subsystems (for common bindings). Since we already do the parsing at
this point, it is a convenient place to trigger the probe of the
dependency. Is ACPI going to be similar in this regard?

Fundamentally, it is a question of probe devices when their
dependencies are present or drivers ensure their dependencies are
ready. IIRC, init systems went thru a similar debate for service
dependencies.

>> In any case, we're talking about adding 1 line.
>
> But also about making the driver core slighly OF-centric.

How so? The one line is in DT binding parsing code in subsystems, not
driver core. The driver core change is we add every device (that
happened to be created by DT) to the deferred probe list, so they
don't probe right away.


> Sure, we need OF-specific code and ACPI-specific code wherever different
> handling is required, but doing that at the driver core level seems to be
> a bit of a stretch to me.
>
> Please note that we don't really have ACPI-specific calls in the driver core,
> although we might have added them long ago even before the OF stuff appeared
> in the kernel for the first time.  We didn't do that, (among other things)
> because we didn't want that particular firmware interface to appear special
> in any 

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Sat, Oct 17, 2015 at 1:57 AM, Greg Kroah-Hartman
 wrote:
> On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
>> Hi Rob,
>>
>> here is the pull request you asked for, with no changes from the version
>> that I posted last to the list.
>>
>> The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
>>
>> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
>>
>> are available in the git repository at:
>>
>> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
>> on-demand-probes-for-next
>
> That's not a signed tag :(
>
> Anyway, I REALLY don't like this series (sorry for the delay in
> reviewing them, normally I trust Rob's judgement...)

We've seen a lot of attempts here. This is really the best solution so
far in that it is simple, uses existing data from DT, and was low risk
for breaking platforms (at least I thought it would be). Anyway,
getting more exposure is why I've put it into -next.

> I can't see adding calls like this all over the tree just to solve a
> bus-specific problem, you are adding of_* calls where they aren't
> needed, or wanted, at all.

I think Linus W, Mark B, and I all said a similar thing initially in
that dependencies should be handled in the driver core. We went down
the path of making this not firmware (aka bus) specific and an earlier
version had just that (with fwnode_* calls). That turned out to be
pointless as the calling locations were almost always in DT specific
code anyway. If you notice, the calls are next to other DT specific
calls generally (usually a "get"). So yes, I'd prefer not to have to
touch every subsystem, but we had to do that anyway to add DT support.

We've generally split the DT code into the core (in drivers/of) and
the binding specific (in subsystems). Extracting dependency
information the DT is going to require binding specific knowledge, so
subsystem changes are probably unavoidable.

The alternative is we put binding specific knowledge into the core DT
code to parse dependencies.

> What is the root-problem of your delay in device probing?  I read your
> last patch series and I can't seem to figure out what the issue is that
> this is solving in any "better" way from the existing deferred probing.

It saves 2 seconds in the boot time as re-probing takes time. That
alone seems compelling to me.

Another downside to deferred probing is you have to touch every driver
and subsystem to support it. This contains the problem to the
subsystems.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Sat, Oct 17, 2015 at 10:47 AM, Greg Kroah-Hartman
<gre...@linuxfoundation.org> wrote:
> On Sat, Oct 17, 2015 at 10:04:55AM -0500, Rob Herring wrote:
>> On Sat, Oct 17, 2015 at 1:57 AM, Greg Kroah-Hartman
>> <gre...@linuxfoundation.org> wrote:
>> > On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:
>> >> Hi Rob,
>> >>
>> >> here is the pull request you asked for, with no changes from the version
>> >> that I posted last to the list.
>> >>
>> >> The following changes since commit 
>> >> 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:
>> >>
>> >> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)
>> >>
>> >> are available in the git repository at:
>> >>
>> >> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
>> >> on-demand-probes-for-next
>> >
>> > That's not a signed tag :(
>> >
>> > Anyway, I REALLY don't like this series (sorry for the delay in
>> > reviewing them, normally I trust Rob's judgement...)
>>
>> We've seen a lot of attempts here. This is really the best solution so
>> far in that it is simple, uses existing data from DT, and was low risk
>> for breaking platforms (at least I thought it would be). Anyway,
>> getting more exposure is why I've put it into -next.
>
> Exposure is good, now we know it breaks some builds, which was useful :)

Now that I've looked at them, they are somewhat questionable failures.
They do show the fragile nature of probe ordering and the implicit
dependencies we have.

>> > I can't see adding calls like this all over the tree just to solve a
>> > bus-specific problem, you are adding of_* calls where they aren't
>> > needed, or wanted, at all.
>>
>> I think Linus W, Mark B, and I all said a similar thing initially in
>> that dependencies should be handled in the driver core. We went down
>> the path of making this not firmware (aka bus) specific and an earlier
>> version had just that (with fwnode_* calls). That turned out to be
>> pointless as the calling locations were almost always in DT specific
>> code anyway. If you notice, the calls are next to other DT specific
>> calls generally (usually a "get"). So yes, I'd prefer not to have to
>> touch every subsystem, but we had to do that anyway to add DT support.
>
> If they are "next" to a call like that, why not put it in that call?  I
> really object to having to "sprinkle" this all over the kernel, for no
> obvious reason why that is happening at all (look at the USB patch for
> one such example.)

Looking at it again, they are in DT specific code already. The USB one
is in devm_usb_get_phy_by_node() which is a DT specific call.

>> We've generally split the DT code into the core (in drivers/of) and
>> the binding specific (in subsystems). Extracting dependency
>> information the DT is going to require binding specific knowledge, so
>> subsystem changes are probably unavoidable.
>>
>> The alternative is we put binding specific knowledge into the core DT
>> code to parse dependencies.
>>
>> > What is the root-problem of your delay in device probing?  I read your
>> > last patch series and I can't seem to figure out what the issue is that
>> > this is solving in any "better" way from the existing deferred probing.
>>
>> It saves 2 seconds in the boot time as re-probing takes time. That
>> alone seems compelling to me.
>
> 2 seconds is _forever_, and really seems like some other driver is
> sleeping and causing this problem.  What does the bootlog time-chart say
> is really causing this long delay?  There's no way we are stuck in some
> sort of logic loop for that long (i.e. having to walk the list of
> devices somehow.)  This sounds like a driver-specific problem that is
> being worked around by having to touch all subsystems, which isn't nice.

I don't think it is one driver as the improvement is seen on multiple
platforms. I'll let Tomeu comment further on where the time was spent.

> Hint, we didn't have to do this type of thing to solve boot delays on
> x86 when we had hardware that was slow to initialize, why should DT be
> special?  :)

x86 did not need deferred probe either (though we probably can find
some initcall ordering hacks). This is an embedded problem, not a DT
problem.

I'm guessing the time is a matter of probing and undoing the probes
rather than slow h/w. We could maybe improve things by making sure
drivers move what they defer on to the beginning of probe, but that
seems like a horrible, fragile hack.

>> Another downside to deferred probing is you ha

Re: [GIT PULL] On-demand device probing

[Rob Herring]

On Fri, Oct 16, 2015 at 4:23 PM, Olof Johansson  wrote:
> Hi,
>
> I've bisected boot failures in next-20151016 down to patches in this branch:
>
> On Thu, Oct 15, 2015 at 4:42 AM, Tomeu Vizoso
>  wrote:
>> Tomeu Vizoso (20):
>>   driver core: handle -EPROBE_DEFER from bus_type.match()
>
> The machine it happened on was OMAP5UEVM:
>
> http://arm-soc.lixom.net/bootlogs/next/next-20151016/omap5uevm-arm-omap2plus_defconfig.html

So this one is because the MMC node numbering changed. I don't know
how to fix that other than with aliases, but that doesn't solve
backwards compatibility.


> But I've also seen it on tegra2, that one bisected down to:
>
>>  regulator: core: Probe regulators on demand
>
> http://arm-soc.lixom.net/bootlogs/next/next-20151016/seaboard-arm-multi_v7_defconfig.html

This one you need a rootwait I think. The MMC scanning is not
guaranteed to be done before the rootfs mounting AFAIK. There may be
other problems, but we can't see them since it panics.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/22] On-demand device probing

[Rob Herring]

On Mon, Sep 7, 2015 at 7:23 AM, Tomeu Vizoso  wrote:
> Hello,
>
> I have a problem with the panel on my Tegra Chromebook taking longer
> than expected to be ready during boot (Stéphane Marchesin reported what
> is basically the same issue in [0]), and have looked into ordered
> probing as a better way of solving this than moving nodes around in the
> DT or playing with initcall levels and linking order.
>
> While reading the thread [1] that Alexander Holler started with his
> series to make probing order deterministic, it occurred to me that it
> should be possible to achieve the same by probing devices as they are
> referenced by other devices.
>
> This basically reuses the information that is already implicit in the
> probe() implementations, saving us from refactoring existing drivers or
> adding information to DTBs.
>
> During review of v1 of this series Linus Walleij suggested that it
> should be the device driver core to make sure that dependencies are
> ready before probing a device. I gave this idea a try [2] but Mark Brown
> pointed out to the logic duplication between the resource acquisition
> and dependency discovery code paths (though I think it's fairly minor).
>
> To address that code duplication I experimented with Arnd's devm_probe
> [3] concept of having drivers declare their dependencies instead of
> acquiring them during probe, and while it worked [4], I don't think we
> end up winning anything when compared to just probing devices on-demand
> from resource getters.
>
> One remaining objection is to the "sprinkling" of calls to
> of_device_probe() in the resource getters of each subsystem, but I think
> it's the right thing to do given that the storage of resources is
> currently subsystem-specific.
>
> We could avoid the above by moving resource storage into the core, but I
> don't think there's a compelling case for that.
>
> I have tested this on boards with Tegra, iMX.6, Exynos, Rockchip and
> OMAP SoCs, and these patches were enough to eliminate all the deferred
> probes (except one in PandaBoard because omap_dma_system doesn't have a
> firmware node as of yet).
>
> Have submitted a branch [5] with only these patches on top of thursday's
> linux-next to kernelci.org and I don't see any issues that could be
> caused by them. For some reason it currently has more passes than the
> version of -next it's based on!
>
> With this series I get the kernel to output to the panel in 0.5s,
> instead of 2.8s.
>
> Regards,
>
> Tomeu
>
> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>
> [1] https://lkml.org/lkml/2014/5/12/452
>
> [2] https://lkml.org/lkml/2015/6/17/305
>
> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
>
> [4] https://lkml.org/lkml/2015/7/21/441a
>
> [5] 
> https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v6
>
> [6] 
> http://kernelci.org/boot/all/job/collabora/kernel/v4.2-11902-g25d80c927f8b/
>
> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150903/
>
> Changes in v4:
> - Added bus.pre_probe callback so the probes of Primecell devices can be
>   deferred if their device IDs cannot be yet read because of the clock
>   driver not having probed when they are registered. Maybe this goes
>   overboard and the matching information should be in the DT if there is
>   one.

Seems overboard to me or at least a separate problem. Most clocks have
to be setup before the driver model simply because timers depend on
clocks usually.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 9/9] dts: tegra: WIP: hack dts to test new dt flags for i2c

[Rob Herring]

On Mon, Jul 20, 2015 at 3:45 AM, Wolfram Sang w...@the-dreams.de wrote:

  +
  +   eeprom@42 {
  +   compatible = linux,slave-24c02;
  +   //FIXME: Should be I2C_OWN_SLAVE_ADDRESS | 0x42
  +   reg = 0xc042;

 The node name doesn't match the reg property anymore. Isn't that considered 
 as
 a problem ?

 Hmm, true. So far, Rob (CCed) was fine with this approach:
 http://www.spinics.net/lists/linux-tegra/msg22760.html

 @Rob: If we introduce flag bits in the MSBs of an I2C address, the reg
 property is different from the node name. Is this a problem?

No, I don't it is a problem.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/3] of: make of_mutex public

[Rob Herring]

+Pantelis

On Tue, Jun 30, 2015 at 4:44 PM, Wolfram Sang w...@the-dreams.de wrote:
 From: Wolfram Sang wsa+rene...@sang-engineering.com

 If we want to use OF_DYNAMIC features outside the of framework, we need
 to access this lock. If OF maintainers don't like exporting, we can
 maybe create accessor functions that we can use?

It looks like you are just taking the mutex around various
of_changeset_* calls. We should either split the mutex into of_mutex
for core and changesets and an overlay mutex for overlays, or we need
to do the typical pattern of having changeset functions having locked
and unlocked versions for external and internal (overlays) use.

Rob

 Signed-off-by: Wolfram Sang wsa+rene...@sang-engineering.com
 ---
  drivers/of/of_private.h | 1 -
  include/linux/of.h  | 2 ++
  2 files changed, 2 insertions(+), 1 deletion(-)

 diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
 index 8e882e706cd8c6..f92ec41efb5dfd 100644
 --- a/drivers/of/of_private.h
 +++ b/drivers/of/of_private.h
 @@ -31,7 +31,6 @@ struct alias_prop {
 char stem[0];
  };

 -extern struct mutex of_mutex;
  extern struct list_head aliases_lookup;
  extern struct kset *of_kset;

 diff --git a/include/linux/of.h b/include/linux/of.h
 index b871ff9d81d720..f0464efcbdc5aa 100644
 --- a/include/linux/of.h
 +++ b/include/linux/of.h
 @@ -32,6 +32,8 @@
  typedef u32 phandle;
  typedef u32 ihandle;

 +extern struct mutex of_mutex;
 +
  struct property {
 char*name;
 int length;
 --
 2.1.4

 --
 To unsubscribe from this list: send the line unsubscribe devicetree in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/21] On-demand device registration

[Rob Herring]

On Mon, Jun 22, 2015 at 10:23 AM, Tomeu Vizoso
tomeu.viz...@collabora.com wrote:
 On 28 May 2015 at 06:33, Rob Herring robherri...@gmail.com wrote:
 On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso
 tomeu.viz...@collabora.com wrote:
 Hello,

 I have a problem with the panel on my Tegra Chromebook taking longer than
 expected to be ready during boot (Stéphane Marchesin reported what is
 basically the same issue in [0]), and have looked into ordered probing as a
 better way of solving this than moving nodes around in the DT or playing 
 with
 initcall levels.

 While reading the thread [1] that Alexander Holler started with his series 
 to
 make probing order deterministic, it occurred to me that it should be 
 possible
 to achieve the same by registering devices as they are referenced by other
 devices.

 I like the concept and novel approach.

 This basically reuses the information that is already implicit in the 
 probe()
 implementations, saving us from refactoring existing drivers or adding
 information to DTBs.

 Something I'm not completely happy with is that I have had to move the call 
 to
 of_platform_populate after all platform drivers have been registered.
 Otherwise I don't see how I could register drivers on demand as we don't 
 have
 yet each driver's compatible strings.

 Yeah, this is the opposite of what we'd really like.

 Can you elaborate on the reasons why we would like to have devices
 registered before built-in drivers finish registering, even if we
 don't probe them yet?

My main thought was for modules we will almost always have devices
appearing first. More generally, we can have devices and drivers
coming or going at any point in time and should not put restrictions
on ordering.

Also, I think all the probe ordering and dependency tracking should be
done within the driver core (i.e. dependencies are a list of struct
devices). At some level it has to become firmware specific, but we
want to minimize that part. I could be convinced otherwise and you
have put more thought into this problem than I have.

 Ideally, we would
 have a solution that works for modules too. However, we're no worse
 off. We pretty much build-in dependencies to avoid module ordering
 problems.

 Nod, I haven't looked yet at requesting modules on-demand, but I guess
 it should be doable. Modules that have dependencies described in the
 firmware should get them probed automatically already though.

 Perhaps we need to make the probing on-demand rather than simply on
 device-driver match occurring.

 I'm afraid that too much old code depends on that. For example, Rafael
 pointed out to the PNP subsystem, which registers a driver that will
 probe devices with the EISA ID PNP0c02 to reserve memory regions for
 devices that will be probed later.

 http://lxr.free-electrons.com/source/drivers/pnp/system.c

 My understanding is that probing of PNP0c02 devices must happen before
 the actual devices that depend on those regions are probed, so if we
 decoupled the probing from the driver/device registration, we would be
 breaking that assumption.

That shouldn't matter as PNP matching is PNP specific. We already have
different ways of matching with device/driver name or of_match_table
for example. Changing how and when OF matching occurs would not affect
PNP matching. We do matching on device and driver add currently. For
the when part, we would need to add what I'll call async matching or
deferred matching which in addition to matching on the of_match_table
also matches on the dependency list having probed. Your last series
essentially does this, but the difference is yours is not OF specific
and I think it needs to be. I mean it is OF specific only in the
aspect that matching already is. From a driver and subsystem
standpoint, it should not be OF specific much like deferred probe is
not OF specific, but in reality only occurs (currently) on OF probed
drivers.

 For machs that don't move of_platform_populate() to a later point, these
 patches shouldn't cause any problems but it's not guaranteed that we'll 
 avoid
 all the deferred probes as some drivers may not be registered yet.

 Ideally, of_platform_populate is not explicitly called by each
 platform. So I think we need to make this work for the default case.

 The problem is that some platforms will need fixing because some
 initcalls assume that some devices will have been registered already,
 or even probed. I think removing those assumptions shouldn't be
 problematic because I haven't had much trouble with this on the four
 platforms I have tested with, but I cannot test every board that is
 supported upstream.

 I can ask though the KernelCI folks to boot my branch in all their
 boards and make sure that those work when of_platform_populate is
 called in late_initcall.

I'd imagine Kevin would be happy to. That is still a subset of h/w, so
we'd need a way to disable any solution.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in

Re: [PATCH-V2 12/12] Documentation: binding: add sclk adjustment properties to i2c-pxa

[Rob Herring]

On Mon, Jun 15, 2015 at 10:49 AM, Vaibhav Hiremath
vaibhav.hirem...@linaro.org wrote:
 With addition of PXA910 family of devices, the TWSI module supports
 new feature which allows us to adjust SCLK.
 With DT properties i2c-pxa driver takes input configuration
 in nsec and converts it to respective bit-fields,

  - i2c-sclk-low-time-ns : SCLK low time (tlow)
This property is used along with mode selection.
  - i2c-sclk-high-time-ns : SCLK high time (thigh)

 Signed-off-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org
 ---
  Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 13 +
  1 file changed, 13 insertions(+)

 diff --git a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
 index 9657db5..0fafd91 100644
 --- a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
 +++ b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
 @@ -23,12 +23,25 @@ Optional properties :
   - i2c-disable-after-xfer : If set, driver will disable I2C module
 after msg xfer and enable it again before xfer.

 +   (Applicable to PXA910 family):
 +
 + - i2c-sclk-low-time-ns : SCLK low time (tlow), for standard/fast/high
 +   speed mode.
 +   This property is used along with mode selection. Driver uses this property
 +   to set low/high time for standard and fast speed mode, as counter 
 bit-field
 +   is same for both.

This belongs below both properties, and it should be clear that both
properties are required if present.

 + - i2c-sclk-high-time-ns : SCLK high time (thigh), Used in case of high speed
 +   mode.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: Mark instantiated device nodes with OF_POPULATE

[Rob Herring]

On Wed, Jun 10, 2015 at 12:40 AM, Wolfram Sang w...@the-dreams.de wrote:

  Mark (and unmark) device nodes with the POPULATE flag as appropriate.
  This is required to avoid multi probing when using I2C and device
  overlays containing a mux.
  This patch is also more careful with the release of the adapter device
  which caused a deadlock with muxes, and does not break the build
  on !OF since the node flag accessors are not defined then.
 
  Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
 
  Now, that the dependency is upstream: applied to for-current, thanks!
 
  I'm not seeing this in linux-next, or in your for-current branch.
 
  Was this dropped or superseded by something else?
 
  I dropped it because it caused a build bug. So, we need another
  dependency in... I am not sure if this is already in.
 

 FWIW, I’ve posted the dependency patch already; it’s trivial as you recall.

 Lets hope it gets picked so that this one can go in.

 Is that in now?

You mean of: Move OF flags to be visible even when !CONFIG_OF? If
so, it is queued up for 4.2 in my tree.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: Mark instantiated device nodes with OF_POPULATE

[Rob Herring]

On Wed, Jun 10, 2015 at 9:08 AM, Wolfram Sang w...@the-dreams.de wrote:
  IIRC it is the one which adds OF_POPULATED. If so, why is this not
  scheduled for 4.1 as this bugfix depends on it?

 No idea. This is an obvious bugfix.

 Dunno if you did but mentioning after the commit message where you think
 it should be applied is very helpful for maintainers.

+1

 I would like to, yes. It’s just the nature of the process when dealing with
 multiple kernel trees. I would be happy for someone to pick up and queue it
 for 4.1.

 That might be too late now... You could also have sent the patch to me
 so that I could apply it via i2c with rob's ack. Next time...

That is exactly what I suggested on Jan 26 and Wolfram asked to resend
with my ack. Pantelis sent it on Apr 24 (w/o my ack) and I applied it
on May 29. Wolfram acked it, so I assumed you were not picking it up.
It sat there for over a month, so it must not have been critical to
apply for 4.1.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: Mark instantiated device nodes with OF_POPULATE

[Rob Herring]

On Wed, Jun 10, 2015 at 9:10 AM, Pantelis Antoniou
pantelis.anton...@konsulko.com wrote:
 Hi Wolfram,

 On Jun 10, 2015, at 17:08 , Wolfram Sang w...@the-dreams.de wrote:

 IIRC it is the one which adds OF_POPULATED. If so, why is this not
 scheduled for 4.1 as this bugfix depends on it?

 No idea. This is an obvious bugfix.

 Dunno if you did but mentioning after the commit message where you think
 it should be applied is very helpful for maintainers.

 I would like to, yes. It’s just the nature of the process when dealing with
 multiple kernel trees. I would be happy for someone to pick up and queue it
 for 4.1.

 That might be too late now... You could also have sent the patch to me
 so that I could apply it via i2c with rob's ack. Next time...


 No worries. I’ve waited a couple of years until these things got in, I can 
 wait
 one more release cycle.

You should know this already, but in the future for cross subsystem
dependencies:
- send the whole series together
- Be clear if they are targeted for current rc and/or stable
- suggest who you think should merge them:
  - for fixes, whomever took the offending commit if it is the same
kernel cycle.
  - for dependencies, get ack's on the dependencies.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/12] i2c: pxa: Add bus reset functionality

[Rob Herring]

On Thu, May 28, 2015 at 8:03 AM, Vaibhav Hiremath
vaibhav.hirem...@linaro.org wrote:
 From: Rob Herring r...@kernel.org

This probably should still be Leilei, but...

 Since there is some problematic i2c slave devices on some
 platforms such as dkb (sometimes), it will drop down sda
 and make i2c bus hang, at that time, it need to config
 scl/sda into gpio to simulate stop sequence to recover
 i2c bus, so add this interface.

 Signed-off-by: Leilei Shang shan...@marvell.com
 Signed-off-by: Rob Herring r...@kernel.org
 [vaibhav.hirem...@linaro.org: Updated Changelog]
 Signed-off-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org

 Signed-off-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org
 ---
  drivers/i2c/busses/i2c-pxa.c | 90 
 
  1 file changed, 90 insertions(+)

 diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
 index 8ca5552..eb09071 100644
 --- a/drivers/i2c/busses/i2c-pxa.c
 +++ b/drivers/i2c/busses/i2c-pxa.c
 @@ -37,6 +37,8 @@
  #include linux/slab.h
  #include linux/io.h
  #include linux/i2c/pxa-i2c.h
 +#include linux/of_gpio.h
 +#include linux/pinctrl/consumer.h

  #include asm/irq.h

 @@ -177,6 +179,9 @@ struct pxa_i2c {
 boolhighmode_enter;
 unsigned intilcr;
 unsigned intiwcr;
 +   struct pinctrl  *pinctrl;
 +   struct pinctrl_state*pin_i2c;
 +   struct pinctrl_state*pin_gpio;
  };

  #define _IBMR(i2c) ((i2c)-reg_ibmr)
 @@ -269,6 +274,62 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int 
 lno, const char *fname)

  #define show_state(i2c) i2c_pxa_show_state(i2c, __LINE__, __func__)

 +static void i2c_bus_reset(struct pxa_i2c *i2c)

There's a generic mechanism in i2c_generic_gpio_recovery we should use
here. It appears to be similar, but not exactly the same. The pinctrl
part should probably be done by gpio driver.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/21] On-demand device registration

[Rob Herring]

On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso
tomeu.viz...@collabora.com wrote:
 Hello,

 I have a problem with the panel on my Tegra Chromebook taking longer than
 expected to be ready during boot (Stéphane Marchesin reported what is
 basically the same issue in [0]), and have looked into ordered probing as a
 better way of solving this than moving nodes around in the DT or playing with
 initcall levels.

 While reading the thread [1] that Alexander Holler started with his series to
 make probing order deterministic, it occurred to me that it should be possible
 to achieve the same by registering devices as they are referenced by other
 devices.

I like the concept and novel approach.

 This basically reuses the information that is already implicit in the probe()
 implementations, saving us from refactoring existing drivers or adding
 information to DTBs.

 Something I'm not completely happy with is that I have had to move the call to
 of_platform_populate after all platform drivers have been registered.
 Otherwise I don't see how I could register drivers on demand as we don't have
 yet each driver's compatible strings.

Yeah, this is the opposite of what we'd really like. Ideally, we would
have a solution that works for modules too. However, we're no worse
off. We pretty much build-in dependencies to avoid module ordering
problems.

Perhaps we need to make the probing on-demand rather than simply on
device-driver match occurring.

 For machs that don't move of_platform_populate() to a later point, these
 patches shouldn't cause any problems but it's not guaranteed that we'll avoid
 all the deferred probes as some drivers may not be registered yet.

Ideally, of_platform_populate is not explicitly called by each
platform. So I think we need to make this work for the default case.

 I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
 patches were enough to eliminate all the deferred probes.

 With this series I get the kernel to output to the panel in 0.5s, instead of 
 2.8s.

That's certainly compelling.

Rob


 Regards,

 Tomeu

 [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html

 [1] https://lkml.org/lkml/2014/5/12/452

 Tomeu Vizoso (21):
   regulator: core: Reduce critical area in _regulator_get
   ARM: tegra: Add gpio-ranges property
   ARM: tegra: Register drivers before devices
   ARM: EXYNOS: Register drivers before devices
   ARM i.MX6q: Register drivers before devices
   of/platform: Add of_platform_device_ensure()
   of/platform: Ensure device registration on lookup
   gpio: Probe GPIO drivers on demand
   gpio: Probe pinctrl devices on demand
   regulator: core: Probe regulators on demand
   drm: Probe panels on demand
   drm/tegra: Probe dpaux devices on demand
   i2c: core: Probe i2c master devices on demand
   pwm: Probe PWM chip devices on demand
   backlight: Probe backlight devices on demand
   usb: phy: Probe phy devices on demand
   clk: Probe clk providers on demand
   pinctrl: Probe pinctrl devices on demand
   phy: core: Probe phy providers on demand
   dma: of: Probe DMA controllers on demand
   power-supply: Probe power supplies on demand

  arch/arm/boot/dts/tegra124.dtsi |  1 +
  arch/arm/mach-exynos/exynos.c   |  4 +--
  arch/arm/mach-imx/mach-imx6q.c  | 12 -
  arch/arm/mach-tegra/tegra.c | 21 ++-
  drivers/clk/clk.c   |  3 +++
  drivers/dma/of-dma.c|  3 +++
  drivers/gpio/gpiolib-of.c   |  5 
  drivers/gpu/drm/drm_panel.c |  3 +++
  drivers/gpu/drm/tegra/dpaux.c   |  3 +++
  drivers/i2c/i2c-core.c  |  3 +++
  drivers/of/platform.c   | 53 
 +
  drivers/phy/phy-core.c  |  3 +++
  drivers/pinctrl/devicetree.c|  2 ++
  drivers/power/power_supply_core.c   |  3 +++
  drivers/pwm/core.c  |  3 +++
  drivers/regulator/core.c| 45 +++
  drivers/usb/phy/phy.c   |  3 +++
  drivers/video/backlight/backlight.c |  3 +++
  include/linux/of_platform.h |  2 ++
  19 files changed, 130 insertions(+), 45 deletions(-)

 --
 2.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] of: base: add function to get highest id of an alias stem

[Rob Herring]

On Thu, Mar 12, 2015 at 11:17 AM, Wolfram Sang w...@the-dreams.de wrote:
 I2C supports adding adapters using either a dynamic or fixed id. The
 latter is provided by aliases in the DT case. To prevent id collisions
 of those two types, install this function which gives us the highest
 fixed id, so we can then let the dynamically created ones come after
 this highest number.

 Signed-off-by: Wolfram Sang w...@the-dreams.de

Acked-by: Rob Herring r...@kernel.org

 ---

 Because this function is so similar to of_alias_get_id(), I thought of merging
 them into __of_alias_get_id(np, stem, bool get_highest) and have two call
 wrappers but then decided the decreased readability is not worth the hazzle.

  drivers/of/base.c  | 26 ++
  include/linux/of.h |  6 ++
  2 files changed, 32 insertions(+)

 diff --git a/drivers/of/base.c b/drivers/of/base.c
 index 0a8aeb8523fe7d..63cba04aacf686 100644
 --- a/drivers/of/base.c
 +++ b/drivers/of/base.c
 @@ -1958,6 +1958,32 @@ int of_alias_get_id(struct device_node *np, const char 
 *stem)
  }
  EXPORT_SYMBOL_GPL(of_alias_get_id);

 +/**
 + * of_alias_get_highest_id - Get highest alias id for the given stem
 + * @stem:  Alias stem to be examined
 + *
 + * The function travels the lookup table to get the highest alias id for the
 + * given alias stem.  It returns the alias id if found.
 + */
 +int of_alias_get_highest_id(const char *stem)
 +{
 +   struct alias_prop *app;
 +   int id = -ENODEV;
 +
 +   mutex_lock(of_mutex);
 +   list_for_each_entry(app, aliases_lookup, link) {
 +   if (strcmp(app-stem, stem) != 0)
 +   continue;
 +
 +   if (app-id  id)
 +   id = app-id;
 +   }
 +   mutex_unlock(of_mutex);
 +
 +   return id;
 +}
 +EXPORT_SYMBOL_GPL(of_alias_get_highest_id);
 +
  const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,
u32 *pu)
  {
 diff --git a/include/linux/of.h b/include/linux/of.h
 index dfde07e77a632b..9bfcc18ceab3bf 100644
 --- a/include/linux/of.h
 +++ b/include/linux/of.h
 @@ -332,6 +332,7 @@ extern int of_count_phandle_with_args(const struct 
 device_node *np,

  extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align));
  extern int of_alias_get_id(struct device_node *np, const char *stem);
 +extern int of_alias_get_highest_id(const char *stem);

  extern int of_machine_is_compatible(const char *compat);

 @@ -594,6 +595,11 @@ static inline int of_alias_get_id(struct device_node 
 *np, const char *stem)
 return -ENOSYS;
  }

 +static inline int of_alias_get_highest_id(const char *stem)
 +{
 +   return -ENOSYS;
 +}
 +
  static inline int of_machine_is_compatible(const char *compat)
  {
 return 0;
 --
 2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: Mark instantiated device nodes with OF_POPULATE

[Rob Herring]

On Mon, Jan 26, 2015 at 9:55 AM, Wolfram Sang w...@the-dreams.de wrote:
 On Sat, Jan 24, 2015 at 09:16:16AM +0200, Pantelis Antoniou wrote:
 Hi Wolfram,

  On Jan 24, 2015, at 06:26 , Wolfram Sang w...@the-dreams.de wrote:
 
 
  Feel free to apply it for 3.19 with my ack. You then want to pick up
  v1 of this patch.
 
  Sigh, I give up, this is too confusing. I don't know which one is V1,
  they are not numbered in any way. And I have been holding back my pull
  request for too long already.
 
  I'll drop this patch. Whatever should be applied should be resent,
  mentioning the release it is aimed for and how the planning/dependency
  on the DT patch is.
 

 No! This is a bugfix for 3.19 :)

 I’m resending the original patch; the dependency is on

   of: Empty node  property flag accessors when !OF”

 https://lkml.org/lkml/2015/1/21/602

 Which is going in when?

Can you apply it since it is a dependency on this bug fix patch.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: Mark instantiated device nodes with OF_POPULATE

[Rob Herring]

On Thu, Jan 22, 2015 at 9:02 AM, Wolfram Sang w...@the-dreams.de wrote:
 On Thu, Jan 22, 2015 at 08:58:36AM -0600, Rob Herring wrote:
 On Thu, Jan 22, 2015 at 8:48 AM, Wolfram Sang w...@the-dreams.de wrote:
  On Thu, Jan 15, 2015 at 08:33:18PM +0200, Pantelis Antoniou wrote:
  Mark (and unmark) device nodes with the POPULATE flag as appropriate.
  This is required to avoid multi probing when using I2C and device
  overlays containing a mux.
  This patch is also more careful with the release of the adapter device
  which caused a deadlock with muxes, and does not break the build
  on !OF since the node flag accessors are not defined then.
 
  Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
 
  Applied to for-current, thanks!

 Ignoring my comments? It's not a big deal, but Pantelis did send out
 the DT patch necessary for my suggestion[1]. It's needed in another
 place now, too.

 Not sure if I saw this patch before...

 Is this DT patch targetted to for-next or for-current. Because this
 patch is a bugfix and needs to go into this cycle. I'll happily apply a
 cleanup patch to for-next then.

Feel free to apply it for 3.19 with my ack. You then want to pick up
v1 of this patch.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] of: i2c: Add idle-disconnect DT property to PCA954x mux driver

[Rob Herring]

On Thu, Jan 15, 2015 at 6:32 AM, Wolfram Sang w...@the-dreams.de wrote:
 On Fri, Dec 19, 2014 at 06:00:10PM +0100, Alexander Sverdlin wrote:
 of: i2c: Add idle-disconnect DT property to PCA954x mux driver

 Add idle-disconnect device tree property to PCA954x mux driver. The new 
 property
 forces the multiplexer to disconnect child buses in idle state. This is 
 used, for
 example, when there are several multiplexers on the same bus and the devices 
 on
 the underlying buses might have same I2C addresses.

 Basically OK. Question to DT maintainers: idle-disconnect,
 i2c-mux-idle-disconnect, or is there another existing binding we could
 use?

Not that I'm aware of. If this is specific to i2c-mux's then I'd go
with the latter. Either way, this should go in a common i2c or i2c-mux
binding doc.

Alternatively, couldn't the kernel do this automatically when the
above conditions happen?

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: Mark instantiated device nodes with OF_POPULATE

[Rob Herring]

On Thu, Jan 15, 2015 at 12:33 PM, Pantelis Antoniou
pantelis.anton...@konsulko.com wrote:
 Mark (and unmark) device nodes with the POPULATE flag as appropriate.
 This is required to avoid multi probing when using I2C and device
 overlays containing a mux.
 This patch is also more careful with the release of the adapter device
 which caused a deadlock with muxes, and does not break the build
 on !OF since the node flag accessors are not defined then.

Why don't we define them then and get rid of the ifdef below.

It may be good to split this out of of.h because we don't want drivers
fiddling with these bits.

Rob


 Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
 ---
  drivers/i2c/i2c-core.c | 18 +-
  1 file changed, 17 insertions(+), 1 deletion(-)

 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 index 39d25a8..1d44e3a 100644
 --- a/drivers/i2c/i2c-core.c
 +++ b/drivers/i2c/i2c-core.c
 @@ -1122,6 +1122,10 @@ EXPORT_SYMBOL_GPL(i2c_new_device);
   */
  void i2c_unregister_device(struct i2c_client *client)
  {
 +#if IS_ENABLED(CONFIG_OF_DYNAMIC)
 +   if (client-dev.of_node)
 +   of_node_clear_flag(client-dev.of_node, OF_POPULATED);
 +#endif
 device_unregister(client-dev);
  }
  EXPORT_SYMBOL_GPL(i2c_unregister_device);
 @@ -1441,8 +1445,11 @@ static void of_i2c_register_devices(struct i2c_adapter 
 *adap)

 dev_dbg(adap-dev, of_i2c: walking child nodes\n);

 -   for_each_available_child_of_node(adap-dev.of_node, node)
 +   for_each_available_child_of_node(adap-dev.of_node, node) {
 +   if (of_node_test_and_set_flag(node, OF_POPULATED))
 +   continue;
 of_i2c_register_device(adap, node);
 +   }
  }

  static int of_dev_node_match(struct device *dev, void *data)
 @@ -1976,6 +1983,11 @@ static int of_i2c_notify(struct notifier_block *nb, 
 unsigned long action,
 if (adap == NULL)
 return NOTIFY_OK;   /* not for us */

 +   if (of_node_test_and_set_flag(rd-dn, OF_POPULATED)) {
 +   put_device(adap-dev);
 +   return NOTIFY_OK;
 +   }
 +
 client = of_i2c_register_device(adap, rd-dn);
 put_device(adap-dev);

 @@ -1986,6 +1998,10 @@ static int of_i2c_notify(struct notifier_block *nb, 
 unsigned long action,
 }
 break;
 case OF_RECONFIG_CHANGE_REMOVE:
 +   /* already depopulated? */
 +   if (!of_node_check_flag(rd-dn, OF_POPULATED))
 +   return NOTIFY_OK;
 +
 /* find our device by node */
 client = of_find_i2c_device_by_node(rd-dn);
 if (client == NULL)
 --
 1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [1/5] i2c: i2c-davinci: switch to use platform_get_irq

[Rob Herring]

On Fri, Nov 21, 2014 at 5:01 AM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
 On 11/20/2014 11:48 PM, Uwe Kleine-König wrote:
 Hello Grygorii,

 On Thu, Nov 20, 2014 at 12:03:04PM +0200, Grygorii Strashko wrote:
 Switch Davinci I2C driver to use platform_get_irq(), because
 - it is not recommened to use
platform_get_resource(pdev, IORESOURCE_IRQ, ..) for requesting IRQ's
resources any more, as they can be not ready yet in case of DT-booting.
 - it makes code simpler

 CC: Sekhar Nori nsek...@ti.com
 CC: Kevin Hilman khil...@deeprootsystems.com
 CC: Santosh Shilimkar ssant...@kernel.org
 CC: Murali Karicheri m-kariche...@ti.com
 Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
 ---
   drivers/i2c/busses/i2c-davinci.c | 14 +++---
   1 file changed, 7 insertions(+), 7 deletions(-)

 diff --git a/drivers/i2c/busses/i2c-davinci.c 
 b/drivers/i2c/busses/i2c-davinci.c
 index 4d96147..9bbfb8f 100644
 --- a/drivers/i2c/busses/i2c-davinci.c
 +++ b/drivers/i2c/busses/i2c-davinci.c
 @@ -640,13 +640,13 @@ static int davinci_i2c_probe(struct platform_device 
 *pdev)
   {
  struct davinci_i2c_dev *dev;
  struct i2c_adapter *adap;
 -struct resource *mem, *irq;
 -int r;
 +struct resource *mem;
 +int r, irq;

 -irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 -if (!irq) {
 -dev_err(pdev-dev, no irq resource?\n);
 -return -ENODEV;
 +irq = platform_get_irq(pdev, 0);
 One bad thing about platform_get_irq is its unusual handling of irq=0.
 I'm pretty sure you don't want to use this value, so adding something
 like:

   if (!irq)
   irq = -ENXIO

 would be welcome because the usual value for invalid irq is 0 and not
 -ESOMETHING. platform_get_irq is one of the very few functions that
 don't adhere to this convention. With handling = 0 as error your code
 is immune to changes in this area. Although I notice that
 platform_get_irq got worse in this respect to handle -EPROBE_DEFER. hmm.

 Apart from your change I wonder if platform_get_irq should handle
 of_irq_get returning 0 as an error.

 I think you are right and It seems like, the check for !irq should
 be added/restored for OF case in platform_get_irq() too.

Changing the return values of platform_get_irq is tricky as it would
potentially break drivers because NO_IRQ can be 0 or -1 depending on
the arch. Drivers checking against specific values of NO_IRQ would
break. We've done some clean-up in this area, but I suspect more is
needed.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] of/irq: Export of_irq_get()

[Rob Herring]

On Sat, Nov 1, 2014 at 2:20 AM, Wolfram Sang w...@the-dreams.de wrote:
 On Thu, Oct 30, 2014 at 04:17:19PM +0200, Laurent Pinchart wrote:
 On Thursday 30 October 2014 15:16:44 Wolfram Sang wrote:
  On Thu, Oct 30, 2014 at 03:59:36PM +0200, Laurent Pinchart wrote:
   The function will be used by the I2C core which can be compiled as a
   module.
  
   Signed-off-by: Laurent Pinchart
   laurent.pinchart+rene...@ideasonboard.com
 
  I think it makes sense if I take this via I2C to get the dependencies
  for the later patches right?

 It would be easier, yes.

 Oh, DT maintainers are not on CC. Then I can wait pretty long for an
 ack ;) Fixing that.

Acked-by: Rob Herring r...@kernel.org



   ---
  
drivers/of/irq.c | 1 +
1 file changed, 1 insertion(+)
  
   diff --git a/drivers/of/irq.c b/drivers/of/irq.c
   index 1471e0a223a5..0d7765807f49 100644
   --- a/drivers/of/irq.c
   +++ b/drivers/of/irq.c
   @@ -405,6 +405,7 @@ int of_irq_get(struct device_node *dev, int index)
  
 return irq_create_of_mapping(oirq);
  
}
  
   +EXPORT_SYMBOL_GPL(of_irq_get);
  
/**
  
 * of_irq_get_byname - Decode a node's IRQ and return it as a Linux irq
 number

 --
 Regards,

 Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] i2c: Add the ability to match device to compatible string without an of_node

[Rob Herring]

On Wed, Jun 4, 2014 at 7:09 AM, Lee Jones lee.jo...@linaro.org wrote:
 A great deal of I2C devices are currently matched via DT node name, and
 as such the compatible naming convention of 'vendor,device' has gone
 somewhat awry - some nodes don't supply one, some supply an arbitrary
 string and others the correct device name with an arbitrary vendor prefix.

 In an effort to correct this problem we have to supply a mechanism to
 match a device by compatible string AND by simple device name.  This
 function strips off the 'vendor,' part of a supplied compatible string
 and attempts to match without it.

 The plan is to remove this function once all of the compatible strings
 for each device have been brought into line.

Then should you add a warning for cases needing to be fixed?

Rob


 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---
  drivers/i2c/i2c-core.c | 25 +
  include/linux/i2c.h| 10 ++
  2 files changed, 35 insertions(+)

 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 index d3802dc..7dcd5c3 100644
 --- a/drivers/i2c/i2c-core.c
 +++ b/drivers/i2c/i2c-core.c
 @@ -1090,6 +1090,31 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct 
 device_node *node)
 return i2c_verify_adapter(dev);
  }
  EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
 +
 +const struct of_device_id
 +*i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
 + struct device *dev)
 +{
 +   const struct i2c_client *client = i2c_verify_client(dev);
 +   const char *name;
 +
 +   if (!(client  matches))
 +   return NULL;
 +
 +   for (; matches-compatible[0]; matches++) {
 +   name = strchr(matches-compatible, ',');
 +   if (!name)
 +   name = matches-compatible;
 +   else
 +   name++;
 +
 +   if (!strncmp(client-name, name, strlen(client-name)))
 +   return matches;
 +   }
 +
 +   return NULL;
 +}
 +EXPORT_SYMBOL_GPL(i2c_of_match_device_strip_vendor);
  #else
  static void of_i2c_register_devices(struct i2c_adapter *adap) { }
  #endif /* CONFIG_OF */
 diff --git a/include/linux/i2c.h b/include/linux/i2c.h
 index b556e0a..f7ec75b 100644
 --- a/include/linux/i2c.h
 +++ b/include/linux/i2c.h
 @@ -564,6 +564,9 @@ extern struct i2c_client 
 *of_find_i2c_device_by_node(struct device_node *node);
  /* must call put_device() when done with returned i2c_adapter device */
  extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node 
 *node);

 +extern const struct of_device_id
 +*i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
 + struct device *dev);
  #else

  static inline struct i2c_client *of_find_i2c_device_by_node(struct 
 device_node *node)
 @@ -575,6 +578,13 @@ static inline struct i2c_adapter 
 *of_find_i2c_adapter_by_node(struct device_node
  {
 return NULL;
  }
 +
 +const struct of_device_id
 +*i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
 + struct device *dev)
 +{
 +   return NULL;
 +}
  #endif /* CONFIG_OF */

  #endif /* _LINUX_I2C_H */
 --
 1.8.3.2

 --
 To unsubscribe from this list: send the line unsubscribe devicetree in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] i2c: sh_mobile: add devicetree documentation

[Rob Herring]

On Wed, Apr 30, 2014 at 5:32 PM, Geert Uytterhoeven
ge...@linux-m68k.org wrote:
 Hi Wolfram,

 CC devicetree

 On Thu, May 1, 2014 at 12:21 AM, Wolfram Sang w...@the-dreams.de wrote:
 From: Wolfram Sang wsa+rene...@sang-engineering.com

 Reported-by: Geert Uytterhoeven geert+rene...@glider.be
 Signed-off-by: Wolfram Sang wsa+rene...@sang-engineering.com
 ---
  .../devicetree/bindings/i2c/i2c-sh_mobile.txt  | 26 
 ++
  1 file changed, 26 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt

 diff --git a/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt
 new file mode 100644
 index ..d2153ce36fa8
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt
 @@ -0,0 +1,26 @@
 +Device tree configuration for Renesas IIC (sh_mobile) driver
 +
 +Required properties:
 +- compatible  : renesas,iic-soctype. renesas,rmobile-iic as 
 fallback

 Please also list all supported values, so checkpatch can validate DTS 
 compatible
 properties against this binding document.

You could add this pattern to checkpatch as well. I really wish
Renesas would follow normal convention of vendor,chip-device.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/3] i2c: cadence: Document device tree bindings

[Rob Herring]

On Thu, Apr 3, 2014 at 12:59 PM, Soren Brinkmann
soren.brinkm...@xilinx.com wrote:
 Add device tree binding documentation for the Cadence I2C controller.

 Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
 ---

 Changes in v4:
  - moved adding DT docs into this dedicated patch

 Changes in v3: None
 Changes in v2: None

 ---
  .../devicetree/bindings/i2c/i2c-cadence.txt | 21 
 +
  1 file changed, 21 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-cadence.txt

 diff --git a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
 new file mode 100644
 index ..685adf513111
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
 @@ -0,0 +1,21 @@
 +Binding for the Cadence I2C controller
 +
 +Required properties:
 +  compatible: Compatibility string. Must be 'cdns,i2c-r1p10'.
 +  clocks: From common clock bindings. Phandle to input clock.
 +
 +Optional properties:
 +  clock-frequency: Desired operating frequency, in Hz, of the bus (actual may
 +  be lower). Defaults to 40 if not specified.

Since not all devices support 400kHz, I would prefer that the default
be 100kHz. Also, it would be good if what speed the default is
consistent across i2c drivers.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] i2c: qup: Add device tree bindings information

[Rob Herring]

On March 13, 2014 9:07:42 PM CDT, Bjorn Andersson
bjorn.anders...@sonymobile.com wrote:
From: Ivan T. Ivanov iiva...@mm-sol.com

The Qualcomm Universal Peripherial (QUP) wraps I2C mini-core and
provide input and output FIFO's for it. I2C controller can operate
as master with supported bus speeds of 100Kbps and 400Kbps.

Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
[bjorn: reformulated part of binding description
added version to compatible
cleaned up example]
Signed-off-by: Bjorn Andersson bjorn.anders...@sonymobile.com

Acked-by: Rob Herring r...@kernel.org

---
.../devicetree/bindings/i2c/qcom,i2c-qup.txt   |   46

 1 file changed, 46 insertions(+)
create mode 100644
Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt

diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
new file mode 100644
index 000..32ef64e
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
@@ -0,0 +1,46 @@
+Qualcomm Universal Peripheral (QUP) I2C controller
+
+Required properties:
+ - compatible: Should be:
+   * qcom,i2c-qup-v1.1.1 for 8660, 8960 and 8064.
+   * qcom,i2c-qup-v2.1.1 for 8974 v1.
+   * qcom,i2c-qup-v2.2.1 for 8974 v2 and later.
+ - reg: Should contain QUP register address and length.
+ - interrupts: Should contain I2C interrupt.
+
+ - clocks: A list of phandles + clock-specifiers, one for each entry
in
+   clock-names.
+ - clock-names: Should contain:
+   * core for the core clock
+   * iface for the AHB clock
+
+ - #address-cells: Should be 1 Address cells for i2c device address
+ - #size-cells: Should be 0 as i2c addresses have no size component
+
+Optional properties:
+ - clock-frequency: Should specify the desired i2c bus clock frequency
in Hz,
+defaults to 100kHz if omitted.
+
+Child nodes should conform to i2c bus binding.
+
+Example:
+
+ i2c@f9924000 {
+ compatible = qcom,i2c-qup-v2.2.1;
+ reg = 0xf9924000 0x1000;
+ interrupts = 0 96 0;
+
+ clocks = gcc GCC_BLSP1_QUP2_I2C_APPS_CLK, gcc
GCC_BLSP1_AHB_CLK;
+ clock-names = core, iface;
+
+ clock-frequency = 355000;
+
+ #address-cells = 1;
+ #size-cells = 0;
+
+ dummy@60 {
+ compatible = dummy;
+ reg = 0x60;
+ };
+ };
+
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/2] i2c: qup: Add device tree bindings information

[Rob Herring]

On Fri, Jan 17, 2014 at 5:03 PM, Bjorn Andersson
bjorn.anders...@sonymobile.com wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com

 The Qualcomm Universal Peripherial (QUP) wraps I2C mini-core and
 provide input and output FIFO's for it. I2C controller can operate
 as master with supported bus speeds of 100Kbps and 400Kbps.

 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 [bjorn: reformulated part of binding description and cleaned up example]
 Signed-off-by: Bjorn Andersson bjorn.anders...@sonymobile.com
 ---

Patch history?

  .../devicetree/bindings/i2c/qcom,i2c-qup.txt   | 41 
 ++
  1 file changed, 41 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt

 diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt 
 b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
 new file mode 100644
 index 000..a99711b
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
 @@ -0,0 +1,41 @@
 +Qualcomm Universal Peripheral (QUP) I2C controller
 +
 +Required properties:
 + - compatible: Should be qcom,i2c-qup.

Seems a bit generic. All versions of the IP are exactly the same?
qcom,chip-i2c-qup would be better.

 + - reg: Should contain QUP register address and length.
 + - interrupts: Should contain I2C interrupt.
 +
 + - clocks: Should contain the core clock and the AHB clock.
 + - clock-names: Should be core for the core clock and iface for the
 +AHB clock.
 +
 + - #address-cells: Should be 1 Address cells for i2c device address
 + - #size-cells: Should be 0 as i2c addresses have no size component
 +
 +Optional properties:
 + - clock-frequency: Should specify the desired i2c bus clock frequency in Hz,
 +default is 100kHz if omitted.
 +
 +Child nodes should conform to i2c bus binding.
 +
 +Example:
 +
 + i2c2: i2c@f9924000 {
 +   compatible = qcom,i2c-qup;
 +   reg = 0xf9924000 0x1000;
 +   interrupts = 0 96 0;
 +
 +   clocks = gcc_blsp1_qup2_i2c_apps_clk, gcc_blsp1_ahb_clk;
 +   clock-names = core, iface;
 +
 +   clock-frequency = 355000;
 +
 +   #address-cells = 1;
 +   #size-cells = 0;
 +
 +   dummy@60 {
 +   compatible = dummy;
 +   reg = 0x60;
 +   };
 + };
 +
 --
 1.8.2.2

 --
 To unsubscribe from this list: send the line unsubscribe devicetree in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] OF: introduce OF style 'modalias' support for platform bus.

[Rob Herring]

On Thu, Jan 16, 2014 at 1:04 AM, Zhang, Rui rui.zh...@intel.com wrote:


 -Original Message-
 From: Rob Herring [mailto:robherri...@gmail.com]
 Sent: Wednesday, January 15, 2014 9:45 PM
 To: Zhang, Rui
 Cc: linux-ker...@vger.kernel.org; linux-a...@vger.kernel.org; linux-
 i...@vger.kernel.org; linux-...@vger.kernel.org; w...@the-dreams.de; Mark
 Brown; Greg Kroah-Hartman; Wysocki, Rafael J; Grant Likely; Rob Herring;
 jarkko.nik...@linux.intel.com; mika.westerb...@linux.intel.com;
 devicet...@vger.kernel.org
 Subject: Re: [PATCH 4/4] OF: introduce OF style 'modalias' support for
 platform bus.
 Importance: High

 On Tue, Jan 14, 2014 at 2:46 AM, Zhang Rui rui.zh...@intel.com wrote:
  Fix a problem that, the platform bus supports the OF style modalias
 in
  .uevent() call, but not in its device 'modalias' sysfs attribute.
 
  cc: devicet...@vger.kernel.org
  Signed-off-by: Zhang Rui rui.zh...@intel.com

 Acked-by: Rob Herring r...@kernel.org

 As there doesn't appear any dependency with the rest of this series, I
 can take this.

 Thanks.


Looks like there is actually a dependency in modalias_show, and the
comment about PAGE_SIZE would apply here too I think. So the whole
series should be taken together.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] OF: introduce OF style 'modalias' support for platform bus.

[Rob Herring]

On Tue, Jan 14, 2014 at 2:46 AM, Zhang Rui rui.zh...@intel.com wrote:
 Fix a problem that, the platform bus supports the OF style modalias
 in .uevent() call, but not in its device 'modalias' sysfs attribute.

 cc: devicet...@vger.kernel.org
 Signed-off-by: Zhang Rui rui.zh...@intel.com

Acked-by: Rob Herring r...@kernel.org

As there doesn't appear any dependency with the rest of this series, I
can take this.

One minor nit below.

 ---
  drivers/base/platform.c   |4 
  drivers/of/device.c   |3 +++
  include/linux/of_device.h |6 ++
  3 files changed, 13 insertions(+)

 diff --git a/drivers/base/platform.c b/drivers/base/platform.c
 index 2f4aea2..bc78848 100644
 --- a/drivers/base/platform.c
 +++ b/drivers/base/platform.c
 @@ -679,6 +679,10 @@ static ssize_t modalias_show(struct device *dev, struct 
 device_attribute *a,
 struct platform_device  *pdev = to_platform_device(dev);
 int len;

 +   len = of_device_get_modalias(dev, buf, PAGE_SIZE -1);
 +   if (len != -ENODEV)
 +   return len;
 +
 len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);
 if (len != -ENODEV)
 return len;
 diff --git a/drivers/of/device.c b/drivers/of/device.c
 index f685e55..dafb973 100644
 --- a/drivers/of/device.c
 +++ b/drivers/of/device.c
 @@ -85,6 +85,9 @@ ssize_t of_device_get_modalias(struct device *dev, char 
 *str, ssize_t len)
 int cplen, i;
 ssize_t tsize, csize, repend;

 +   if ((!dev) || (!dev-of_node))\

Don't need the parentheses here.

 +   return -ENODEV;
 +
 /* Name  Type */
 csize = snprintf(str, len, of:N%sT%s, dev-of_node-name,
  dev-of_node-type);
 diff --git a/include/linux/of_device.h b/include/linux/of_device.h
 index 82ce324..8d7dd67 100644
 --- a/include/linux/of_device.h
 +++ b/include/linux/of_device.h
 @@ -64,6 +64,12 @@ static inline int of_driver_match_device(struct device 
 *dev,
  static inline void of_device_uevent(struct device *dev,
 struct kobj_uevent_env *env) { }

 +static inline int of_device_get_modalias(struct device *dev,
 +  char *str, ssize_t len)
 +{
 +   return -ENODEV;
 +}
 +
  static inline int of_device_uevent_modalias(struct device *dev,
struct kobj_uevent_env *env)
  {
 --
 1.7.9.5

 --
 To unsubscribe from this list: send the line unsubscribe devicetree in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9] irqdomain: Introduce __irq_create_of_mapping()

[Rob Herring]

On 09/16/2013 03:32 AM, Thierry Reding wrote:
 This is a version of irq_create_of_mapping() that propagates the precise
 error code instead of returning 0 for all errors. It will be used in
 subsequent patches to allow further propagation of error codes.
 
 To avoid code duplication, implement irq_create_of_mapping() as a
 wrapper around the new __irq_create_of_mapping().

This function is a manageable number of callers that the callers should
just be updated and avoid the wrapper.

Rob

 
 Signed-off-by: Thierry Reding tred...@nvidia.com
 ---
  include/linux/of_irq.h |  3 +++
  kernel/irq/irqdomain.c | 39 +--
  2 files changed, 32 insertions(+), 10 deletions(-)
 
 diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
 index 535cecf..c383dd1 100644
 --- a/include/linux/of_irq.h
 +++ b/include/linux/of_irq.h
 @@ -63,6 +63,9 @@ extern int of_irq_map_raw(struct device_node *parent, const 
 __be32 *intspec,
 struct of_irq *out_irq);
  extern int of_irq_map_one(struct device_node *device, int index,
 struct of_irq *out_irq);
 +extern int __irq_create_of_mapping(struct device_node *controller,
 +const u32 *intspec, unsigned int intsize,
 +unsigned int *virqp);
  extern unsigned int irq_create_of_mapping(struct device_node *controller,
 const u32 *intspec,
 unsigned int intsize);
 diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
 index d2a3b01..9867616 100644
 --- a/kernel/irq/irqdomain.c
 +++ b/kernel/irq/irqdomain.c
 @@ -484,39 +484,58 @@ int irq_create_strict_mappings(struct irq_domain 
 *domain, unsigned int irq_base,
  }
  EXPORT_SYMBOL_GPL(irq_create_strict_mappings);
  
 -unsigned int irq_create_of_mapping(struct device_node *controller,
 -const u32 *intspec, unsigned int intsize)
 +int __irq_create_of_mapping(struct device_node *controller, const u32 
 *intspec,
 + unsigned int intsize, unsigned int *virqp)
  {
 + unsigned int type = IRQ_TYPE_NONE;
   struct irq_domain *domain;
   irq_hw_number_t hwirq;
 - unsigned int type = IRQ_TYPE_NONE;
   unsigned int virq;
 + int ret;
  
   domain = controller ? irq_find_host(controller) : irq_default_domain;
   if (!domain) {
   pr_warn(no irq domain found for %s !\n,
   of_node_full_name(controller));
 - return 0;
 + return -EPROBE_DEFER;
   }
  
   /* If domain has no translation, then we assume interrupt line */
   if (domain-ops-xlate == NULL)
   hwirq = intspec[0];
   else {
 - if (domain-ops-xlate(domain, controller, intspec, intsize,
 -  hwirq, type))
 - return 0;
 + ret = domain-ops-xlate(domain, controller, intspec, intsize,
 +  hwirq, type);
 + if (ret)
 + return ret;
   }
  
   /* Create mapping */
 - virq = irq_create_mapping(domain, hwirq);
 - if (!virq)
 - return virq;
 + ret = __irq_create_mapping(domain, hwirq, virq);
 + if (ret)
 + return ret;
  
   /* Set type if specified and different than the current one */
   if (type != IRQ_TYPE_NONE 
   type != irq_get_trigger_type(virq))
   irq_set_irq_type(virq, type);
 +
 + if (virqp)
 + *virqp = virq;
 +
 + return 0;
 +}
 +
 +unsigned int irq_create_of_mapping(struct device_node *controller,
 +const u32 *intspec, unsigned int intsize)
 +{
 + unsigned int virq;
 + int ret;
 +
 + ret = __irq_create_of_mapping(controller, intspec, intsize, virq);
 + if (ret)
 + return 0;
 +
   return virq;
  }
  EXPORT_SYMBOL_GPL(irq_create_of_mapping);
 

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9] of/irq: Introduce of_irq_get()

[Rob Herring]

On 09/16/2013 03:32 AM, Thierry Reding wrote:
 This is a version of irq_of_parse_and_map() that propagates the precise
 error code instead of returning 0 for all errors. It will be used in
 subsequent patches to allow further propagation of error codes.
 
 To avoid code duplication, implement irq_of_parse_and_map() as a wrapper
 around the new of_irq_get().

*_get typically also implies some reference counting which I don't
believe this does. I don't think having 2 functions with completely
different names doing the same thing with only a different calling
convention is good either. So I would keep the old name and the names
aligned.

 Signed-off-by: Thierry Reding tred...@nvidia.com
 ---
  drivers/of/irq.c   | 21 +
  include/linux/of_irq.h |  3 +++
  2 files changed, 20 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/of/irq.c b/drivers/of/irq.c
 index 5f44388..8225289 100644
 --- a/drivers/of/irq.c
 +++ b/drivers/of/irq.c
 @@ -26,6 +26,20 @@
  #include linux/string.h
  #include linux/slab.h
  
 +int of_irq_get(struct device_node *dev, unsigned int index, unsigned int 
 *virqp)
 +{
 + struct of_irq oirq;
 + int ret;
 +
 + ret = of_irq_map_one(dev, index, oirq);
 + if (ret)
 + return ret;
 +
 + return __irq_create_of_mapping(oirq.controller, oirq.specifier,
 +oirq.size, virqp);
 +}
 +EXPORT_SYMBOL_GPL(of_irq_get);
 +
  /**
   * irq_of_parse_and_map - Parse and map an interrupt into linux virq space
   * @dev: Device node of the device whose interrupt is to be mapped
 @@ -36,13 +50,12 @@
   */
  unsigned int irq_of_parse_and_map(struct device_node *dev, int index)
  {
 - struct of_irq oirq;
 + unsigned int virq;
  
 - if (of_irq_map_one(dev, index, oirq))
 + if (of_irq_get(dev, index, virq))
   return 0;
  
 - return irq_create_of_mapping(oirq.controller, oirq.specifier,
 -  oirq.size);
 + return virq;

This can be an inline and written more concisely:

{
unsigned int virq;
return (of_irq_get(dev, index, virq)  0) ? 0 : virq;
}

Rob


--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9] of/irq: Introduce __of_irq_to_resource()

[Rob Herring]

On 09/16/2013 03:32 AM, Thierry Reding wrote:
 This is a version of of_irq_to_resource() that propagates the precise
 error code instead of returning 0 for all errors. It will be used in
 subsequent patches to allow further propagation of error codes.
 
 To avoid code duplication, implement of_irq_to_resource() as a wrapper
 around the new __of_irq_to_resource().

I think the callers in this case are manageable to update as well.
Several cases could simply use irq_of_parse_and_map instead as they just
pass in a NULL resource.

Rob

 
 Signed-off-by: Thierry Reding tred...@nvidia.com
 ---
  drivers/of/irq.c | 33 +
  1 file changed, 25 insertions(+), 8 deletions(-)
 
 diff --git a/drivers/of/irq.c b/drivers/of/irq.c
 index 8225289..57396fd 100644
 --- a/drivers/of/irq.c
 +++ b/drivers/of/irq.c
 @@ -343,15 +343,15 @@ int of_irq_map_one(struct device_node *device, int 
 index, struct of_irq *out_irq
  }
  EXPORT_SYMBOL_GPL(of_irq_map_one);
  
 -/**
 - * of_irq_to_resource - Decode a node's IRQ and return it as a resource
 - * @dev: pointer to device tree node
 - * @index: zero-based index of the irq
 - * @r: pointer to resource structure to return result into.
 - */
 -int of_irq_to_resource(struct device_node *dev, int index, struct resource 
 *r)
 +static int __of_irq_to_resource(struct device_node *dev, unsigned int index,
 + struct resource *r)
  {
 - int irq = irq_of_parse_and_map(dev, index);
 + unsigned int irq;
 + int ret;
 +
 + ret = of_irq_get(dev, index, irq);
 + if (ret)
 + return ret;
  
   /* Only dereference the resource if both the
* resource and the irq are valid. */
 @@ -373,6 +373,23 @@ int of_irq_to_resource(struct device_node *dev, int 
 index, struct resource *r)
  
   return irq;
  }
 +
 +/**
 + * of_irq_to_resource - Decode a node's IRQ and return it as a resource
 + * @dev: pointer to device tree node
 + * @index: zero-based index of the irq
 + * @r: pointer to resource structure to return result into.
 + */
 +int of_irq_to_resource(struct device_node *dev, int index, struct resource 
 *r)
 +{
 + int ret;
 +
 + ret = __of_irq_to_resource(dev, index, r);
 + if (ret  0)
 + return 0;
 +
 + return ret;
 +}
  EXPORT_SYMBOL_GPL(of_irq_to_resource);
  
  /**
 

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND] i2c: move of helpers into the core

[Rob Herring]

On 08/19/2013 12:59 PM, Wolfram Sang wrote:
 I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
 that it is much cleaner to have this in the core. This also removes a
 circular dependency between the helpers and the core, and so we can
 finally register child nodes in the core instead of doing this manually
 in each driver. So, fix the drivers and documentation, too.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de
 ---

Glad to see this.

Acked-by: Rob Herring rob.herr...@calxeda.com

 
 Sigh, hitting the CC threshold on vger again. So resending to the lists only.
 BTW this patch is based on -rc4 and was tested on an AT91 board. More tests
 very welcome. Thanks!
 
 
  Documentation/acpi/enumeration.txt  |1 -
  drivers/i2c/busses/i2c-at91.c   |3 -
  drivers/i2c/busses/i2c-cpm.c|6 --
  drivers/i2c/busses/i2c-davinci.c|2 -
  drivers/i2c/busses/i2c-designware-platdrv.c |2 -
  drivers/i2c/busses/i2c-gpio.c   |3 -
  drivers/i2c/busses/i2c-i801.c   |2 -
  drivers/i2c/busses/i2c-ibm_iic.c|4 -
  drivers/i2c/busses/i2c-imx.c|3 -
  drivers/i2c/busses/i2c-mpc.c|2 -
  drivers/i2c/busses/i2c-mv64xxx.c|3 -
  drivers/i2c/busses/i2c-mxs.c|3 -
  drivers/i2c/busses/i2c-nomadik.c|3 -
  drivers/i2c/busses/i2c-ocores.c |3 -
  drivers/i2c/busses/i2c-octeon.c |3 -
  drivers/i2c/busses/i2c-omap.c   |3 -
  drivers/i2c/busses/i2c-pnx.c|3 -
  drivers/i2c/busses/i2c-powermac.c   |9 +-
  drivers/i2c/busses/i2c-pxa.c|2 -
  drivers/i2c/busses/i2c-s3c2410.c|2 -
  drivers/i2c/busses/i2c-sh_mobile.c  |2 -
  drivers/i2c/busses/i2c-sirf.c   |3 -
  drivers/i2c/busses/i2c-stu300.c |2 -
  drivers/i2c/busses/i2c-tegra.c  |3 -
  drivers/i2c/busses/i2c-versatile.c  |2 -
  drivers/i2c/busses/i2c-wmt.c|3 -
  drivers/i2c/busses/i2c-xiic.c   |3 -
  drivers/i2c/i2c-core.c  |  107 -
  drivers/i2c/i2c-mux.c   |3 -
  drivers/i2c/muxes/i2c-arb-gpio-challenge.c  |1 -
  drivers/i2c/muxes/i2c-mux-gpio.c|1 -
  drivers/i2c/muxes/i2c-mux-pinctrl.c |1 -
  drivers/media/platform/exynos4-is/fimc-is-i2c.c |3 -
  drivers/of/Kconfig  |6 --
  drivers/of/Makefile |1 -
  drivers/of/of_i2c.c |  114 
 ---
  include/linux/i2c.h |   20 
  include/linux/of_i2c.h  |   46 -
  38 files changed, 130 insertions(+), 253 deletions(-)
  delete mode 100644 drivers/of/of_i2c.c
  delete mode 100644 include/linux/of_i2c.h
 
 diff --git a/Documentation/acpi/enumeration.txt 
 b/Documentation/acpi/enumeration.txt
 index d9be7a9..958266e 100644
 --- a/Documentation/acpi/enumeration.txt
 +++ b/Documentation/acpi/enumeration.txt
 @@ -238,7 +238,6 @@ An I2C bus (controller) driver does:
   if (ret)
   /* handle error */
  
 - of_i2c_register_devices(adapter);
   /* Enumerate the slave devices behind this bus via ACPI */
   acpi_i2c_register_devices(adapter);
  
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 6bb839b..fd05930 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -28,7 +28,6 @@
  #include linux/module.h
  #include linux/of.h
  #include linux/of_device.h
 -#include linux/of_i2c.h
  #include linux/platform_device.h
  #include linux/slab.h
  #include linux/platform_data/dma-atmel.h
 @@ -775,8 +774,6 @@ static int at91_twi_probe(struct platform_device *pdev)
   return rc;
   }
  
 - of_i2c_register_devices(dev-adapter);
 -
   dev_info(dev-dev, AT91 i2c bus driver.\n);
   return 0;
  }
 diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
 index 2e1f7eb..b2b8aa9 100644
 --- a/drivers/i2c/busses/i2c-cpm.c
 +++ b/drivers/i2c/busses/i2c-cpm.c
 @@ -42,7 +42,6 @@
  #include linux/dma-mapping.h
  #include linux/of_device.h
  #include linux/of_platform.h
 -#include linux/of_i2c.h
  #include sysdev/fsl_soc.h
  #include asm/cpm.h
  
 @@ -681,11 +680,6 @@ static int cpm_i2c_probe(struct platform_device *ofdev)
   dev_dbg(ofdev-dev, hw routines for %s registered.\n,
   cpm-adap.name);
  
 - /*
 -  * register OF I2C devices
 -  */
 - of_i2c_register_devices(cpm-adap);
 -
   return 0;
  out_shut:
   cpm_i2c_shutdown(cpm);
 diff --git a/drivers/i2c/busses/i2c-davinci.c 
 b

Re: [PATCH REBASE] i2c-designware: make SDA hold time configurable

[Rob Herring]

On 05/14/2013 08:04 AM, Christian Ruppert wrote:
 This patch makes the SDA hold time configurable through device tree.
 
 [rebased to i2c-current/i2c-next/mainline-3.10-rc1]
 
 Signed-off-by: Christian Ruppert christian.rupp...@abilis.com
 Signed-off-by: Pierrick Hascoet pierrick.hasc...@abilis.com
 ---
  .../devicetree/bindings/i2c/i2c-designware.txt |   14 ++
  drivers/i2c/busses/i2c-designware-core.c   |5 +
  drivers/i2c/busses/i2c-designware-core.h   |1 +
  drivers/i2c/busses/i2c-designware-platdrv.c|9 +
  4 files changed, 29 insertions(+), 0 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
 index e42a2ee..21fabe7 100644
 --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
 +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
 @@ -10,6 +10,9 @@ Recommended properties :
  
   - clock-frequency : desired I2C bus clock frequency in Hz.
  
 +Optional properties :
 + - sda-hold-time : should contain the SDA hold time in nanoseconds.

Please specify time units in the property name. Perhaps
i2c-sda-hold-time-ns.

Based on reading the discussion, there is one similar property I have
found: samsung,i2c-sda-delay = 100. I wouldn't copy it as it doesn't
tell the units or what the delay is.

Otherwise, looks fine to me.

Rob

 +
  Example :
  
   i2c@f {
 @@ -20,3 +23,14 @@ Example :
   interrupts = 11;
   clock-frequency = 40;
   };
 +
 + i2c@112 {
 + #address-cells = 1;
 + #size-cells = 0;
 + compatible = snps,designware-i2c;
 + reg = 0x112 0x1000;
 + interrupt-parent = ictl;
 + interrupts = 12 1;
 + clock-frequency = 40;
 + sda-hold-time = 300;
 + };
 diff --git a/drivers/i2c/busses/i2c-designware-core.c 
 b/drivers/i2c/busses/i2c-designware-core.c
 index 21fbb34..91d422c 100644
 --- a/drivers/i2c/busses/i2c-designware-core.c
 +++ b/drivers/i2c/busses/i2c-designware-core.c
 @@ -67,6 +67,7 @@
  #define DW_IC_STATUS 0x70
  #define DW_IC_TXFLR  0x74
  #define DW_IC_RXFLR  0x78
 +#define DW_IC_SDA_HOLD   0x7c
  #define DW_IC_TX_ABRT_SOURCE 0x80
  #define DW_IC_ENABLE_STATUS  0x9c
  #define DW_IC_COMP_PARAM_1   0xf4
 @@ -332,6 +333,10 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
   dw_writel(dev, lcnt, DW_IC_FS_SCL_LCNT);
   dev_dbg(dev-dev, Fast-mode HCNT:LCNT = %d:%d\n, hcnt, lcnt);
  
 + /* Configure SDA Hold Time if required */
 + if (dev-sda_hold_time)
 + dw_writel(dev, dev-sda_hold_time, DW_IC_SDA_HOLD);
 +
   /* Configure Tx/Rx FIFO threshold levels */
   dw_writel(dev, dev-tx_fifo_depth - 1, DW_IC_TX_TL);
   dw_writel(dev, 0, DW_IC_RX_TL);
 diff --git a/drivers/i2c/busses/i2c-designware-core.h 
 b/drivers/i2c/busses/i2c-designware-core.h
 index 9c1840e..33dfec3 100644
 --- a/drivers/i2c/busses/i2c-designware-core.h
 +++ b/drivers/i2c/busses/i2c-designware-core.h
 @@ -88,6 +88,7 @@ struct dw_i2c_dev {
   u32 master_cfg;
   unsigned inttx_fifo_depth;
   unsigned intrx_fifo_depth;
 + u32 sda_hold_time;
  };
  
  #define ACCESS_SWAP  0x0001
 diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
 b/drivers/i2c/busses/i2c-designware-platdrv.c
 index 8ec9133..589f414 100644
 --- a/drivers/i2c/busses/i2c-designware-platdrv.c
 +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
 @@ -34,6 +34,7 @@
  #include linux/sched.h
  #include linux/err.h
  #include linux/interrupt.h
 +#include linux/of.h
  #include linux/of_i2c.h
  #include linux/platform_device.h
  #include linux/pm.h
 @@ -120,6 +121,14 @@ static int dw_i2c_probe(struct platform_device *pdev)
   return PTR_ERR(dev-clk);
   clk_prepare_enable(dev-clk);
  
 + if (pdev-dev.of_node) {
 + u32 ht;
 + u32 ic_clk = dev-get_clk_rate_khz(dev);
 +
 + of_property_read_u32(pdev-dev.of_node, sda-hold-time, ht);
 + dev-sda_hold_time = ((u64)ic_clk * ht + 50) / 100;
 + }
 +
   dev-functionality =
   I2C_FUNC_I2C |
   I2C_FUNC_10BIT_ADDR |
 

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of_i2c: Honour status=disabled property of device

[Rob Herring]

On 11/28/2012 10:51 AM, Alexander Sverdlin wrote:
 Hi!
 
 On 11/28/2012 05:16 PM, Stephen Warren wrote:
 On 11/28/2012 07:21 AM, Alexander Sverdlin wrote:
 of_i2c: Honour status=disabled property of device

 Currently of_i2c_register_devices() function registers all i2c devices,
 independently from their status property in device tree. According to
 ePAPR 1.1 spec, device should only be registered if there is no
 status property, or it has ok (or okay) value (see
 of_device_is_available()). In case of platform devices,
 of_platform_device_create_pdata() checks for status and ensures
 that disabled devices are not populated. But such check for i2c buses
 was missing until now. Fix it.

 Do SPI and other buses (e.g. MDIO/MDIO mux?) need a similar change?
 
 You are right, of_register_spi_devices() and of_mdiobus_register() both have 
 exactly the same issue.
 Should it be a follow-up patch, or better to combine them to one patch?

Separate please in case there is some reason to revert. Also, I already
have a patch for i2c that does the same thing in my tree. However, your
patch is better, so I'll probably replace it with yours.

Rob

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND] i2c: Add support for device-tree based chip initialization

[Rob Herring]

On 11/25/2012 10:53 PM, Guenter Roeck wrote:
 Some I2C devices are not or not correctly initialized by the firmware.
 Configuration would be possible via platform data, but that would require
 per-driver platform data and a lot of code, and changing it would not be
 possible without re-compiling the kernel. It is more elegant to do it
 generically via devicetree properties.
 
 Add a generic I2C devicetree property named reg-init. This property provides
 a sequence of device initialization commands to be executed prior to calling
 the probe function for a given device.
 
 Signed-off-by: Guenter Roeck li...@roeck-us.net
 ---
 Any comments / feedback ?

This has been discussed before in terms of memory mapped registers. I
think this is of questionable use of DT and could easily be abused. Not
all systems use DT, so this needs to be solved without DT anyway.

Do you have examples of drivers that would use this?

Can this be handled in userspace using the i2c device interface before
loading the device's module?

Rob

 
  .../devicetree/bindings/i2c/trivial-devices.txt|   24 +++
  drivers/i2c/i2c-core.c |   68 
 
  2 files changed, 92 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt 
 b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
 index 2f5322b..33b694e 100644
 --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
 +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
 @@ -2,6 +2,30 @@ This is a list of trivial i2c devices that have simple 
 device tree
  bindings, consisting only of a compatible field, an address and
  possibly an interrupt line.
  
 +Device initialization is supported with an optional reg-init property.
 +This property contains a sequence of commands to be written into the chip.
 +Each command consists of four values: Register, command type, mask, and data.
 + Register:
 + Register or command address
 + Command type:
 + 0: SMBus write byte
 + 1: SMBus write byte data
 + 2: SMBus write word data
 + Mask:
 + If set, the register is read and masked with this value.
 + Data:
 + Data to be written (or with original data and mask if set)
 +
 +Example:
 + max6696@1a {
 + compatible = maxim,max6696;
 + reg = 0x1a;
 + reg-init = 
 + 0x09 1 0x00 0x24
 + 0x21 1 0x00 0x05
 + ;
 + };
 +
  If a device needs more specific bindings, such as properties to
  describe some aspect of it, there needs to be a specific binding
  document for it just like any other devices.
 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 index a7edf98..49f8b74 100644
 --- a/drivers/i2c/i2c-core.c
 +++ b/drivers/i2c/i2c-core.c
 @@ -104,6 +104,69 @@ static int i2c_device_uevent(struct device *dev, struct 
 kobj_uevent_env *env)
  #define i2c_device_ueventNULL
  #endif   /* CONFIG_HOTPLUG */
  
 +static int i2c_dev_of_init(struct i2c_client *client, struct device *dev)
 +{
 + const u32 *reg_init;
 + int rlen;
 + int val;
 + u32 reg, access, mask, data;
 +
 + reg_init = of_get_property(dev-of_node, reg-init, rlen);
 + if (!reg_init)
 + return 0;
 +
 + if (!rlen || rlen % 4)
 + return -EINVAL;
 +
 + while (rlen = 4) {
 + reg = reg_init[0];
 + access = reg_init[1];
 + mask = reg_init[2];
 + data = reg_init[3];
 +
 + if (reg  0xff)
 + return -EINVAL;
 +
 + switch (access) {
 + default:
 + return -EINVAL;
 + case 0:
 + val = 0;
 + break;
 + case 1:
 + val = mask ? i2c_smbus_read_byte_data(client, reg) : 0;
 + break;
 + case 2:
 + val = mask ? i2c_smbus_read_word_data(client, reg) : 0;
 + break;
 + }
 + if (val  0)
 + return val;
 +
 + val = mask;
 + val |= data;
 +
 + switch (access) {
 + default:
 + case 0:
 + val = i2c_smbus_write_byte(client, reg);
 + break;
 + case 1:
 + val = i2c_smbus_write_byte_data(client, reg, val);
 + break;
 + case 2:
 + val = i2c_smbus_write_word_data(client, reg, val);
 + break;
 + }
 + if (val  0)
 + return val;
 +
 + reg_init += 4;
 + rlen -= 4 * sizeof(u32);
 + }
 + return 0;
 +}
 +
  static int i2c_device_probe(struct device *dev)
  {
   struct i2c_client   *client = i2c_verify_client(dev);
 @@ -122,7 +185,12 @@ static int 

Re: [PATCH RESEND] i2c: Add support for device-tree based chip initialization

[Rob Herring]

On 11/26/2012 02:19 PM, Guenter Roeck wrote:
 On Mon, Nov 26, 2012 at 10:43:38AM -0600, Rob Herring wrote:
 On 11/25/2012 10:53 PM, Guenter Roeck wrote:
 Some I2C devices are not or not correctly initialized by the firmware.
 Configuration would be possible via platform data, but that would require
 per-driver platform data and a lot of code, and changing it would not be
 possible without re-compiling the kernel. It is more elegant to do it
 generically via devicetree properties.

 Add a generic I2C devicetree property named reg-init. This property 
 provides
 a sequence of device initialization commands to be executed prior to calling
 the probe function for a given device.

 Signed-off-by: Guenter Roeck li...@roeck-us.net
 ---
 Any comments / feedback ?

 This has been discussed before in terms of memory mapped registers. I
 think this is of questionable use of DT and could easily be abused. Not
 
 Isn't that true for pretty much everything ?
 
 Really, I don't think that it can be abused should be considered a valid
 argument. It is almost as good (or bad) as we have always done it that way
 or this doesn't scale.

DT is a description of the h/w. It is not a h/w configuration mechanism.
Although, you do lots of h/w configuration based on the h/w description.
That being said, you can find examples in bindings that are just
configuration data so it's not a clear line.

If we do want to support this, then there is no reason for it to be
specific to i2c devices or ethernet phys. It was on the pinmux/pinctrl
binding discussions the last time this came up IIRC. The first issue
will be the need to specify register sizes. Then you get into needing
masked writes. Then you need delays and polling reads of register
values. Then the discussion dies when it starts to look like a scripting
language and Forth is mentioned (happened just last week with runtime
interpreted power sequences thread).

Rob

 all systems use DT, so this needs to be solved without DT anyway.

 Do you have examples of drivers that would use this?

 I would need it for MAX6697 (for which I submitted a driver a week or so ago),
 and possibly for others. I didn't check further because I don't want to go 
 along
 on this road too far if the idea is rejected.
 
 I took the idea from Broadcom and Marvell PHY chip initialization, which uses
 a similar approach, including the reg-init keyword. As far as I can see
 both don't support platform data based initialization, so one question
 to ask would be when it is mandatory to support platforma data based
 initialization and when it isn't.

 Can this be handled in userspace using the i2c device interface before
 loading the device's module?

 Not in my use case.
 
 Thanks,
 Guenter
 
 Rob


  .../devicetree/bindings/i2c/trivial-devices.txt|   24 +++
  drivers/i2c/i2c-core.c |   68 
 
  2 files changed, 92 insertions(+)

 diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt 
 b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
 index 2f5322b..33b694e 100644
 --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
 +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
 @@ -2,6 +2,30 @@ This is a list of trivial i2c devices that have simple 
 device tree
  bindings, consisting only of a compatible field, an address and
  possibly an interrupt line.
  
 +Device initialization is supported with an optional reg-init property.
 +This property contains a sequence of commands to be written into the chip.
 +Each command consists of four values: Register, command type, mask, and 
 data.
 +   Register:
 +   Register or command address
 +   Command type:
 +   0: SMBus write byte
 +   1: SMBus write byte data
 +   2: SMBus write word data
 +   Mask:
 +   If set, the register is read and masked with this value.
 +   Data:
 +   Data to be written (or with original data and mask if set)
 +
 +Example:
 +   max6696@1a {
 +   compatible = maxim,max6696;
 +   reg = 0x1a;
 +   reg-init = 
 +   0x09 1 0x00 0x24
 +   0x21 1 0x00 0x05
 +   ;
 +   };
 +
  If a device needs more specific bindings, such as properties to
  describe some aspect of it, there needs to be a specific binding
  document for it just like any other devices.
 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 index a7edf98..49f8b74 100644
 --- a/drivers/i2c/i2c-core.c
 +++ b/drivers/i2c/i2c-core.c
 @@ -104,6 +104,69 @@ static int i2c_device_uevent(struct device *dev, 
 struct kobj_uevent_env *env)
  #define i2c_device_uevent  NULL
  #endif /* CONFIG_HOTPLUG */
  
 +static int i2c_dev_of_init(struct i2c_client *client, struct device *dev)
 +{
 +   const u32 *reg_init;
 +   int rlen;
 +   int val;
 +   u32 reg, access, mask, data;
 +
 +   reg_init = of_get_property(dev-of_node, reg-init, rlen);
 +   if (!reg_init)
 +   return 0

Re: [PATCH v2 1/2] i2c: i2c-ocores: Add irq support for sparc

[Rob Herring]

On 11/13/2012 05:10 AM, Wolfram Sang wrote:
 Hi,
 
 On Mon, Nov 12, 2012 at 05:59:50PM +0100, Andreas Larsson wrote:
 Add sparc support by using platform_get_irq instead of platform_get_resource.
 There are no platform resources of type IORESOURCE_IRQ for sparc, but
 platform_get_irq works for sparc. In the non-sparc case platform_get_irq
 internally uses platform_get_resource.

 Signed-off-by: Andreas Larsson andr...@gaisler.com
 Acked-by: Peter Korsgaard jac...@sunsite.dk
 ---
 Changes since v1:
  - platform_get_irq now works for sparc, so that is used for all platforms
  - Acked by Peter Korsgaard jac...@sunsite.dk

  drivers/i2c/busses/i2c-ocores.c |9 +
  1 files changed, 5 insertions(+), 4 deletions(-)

 diff --git a/drivers/i2c/busses/i2c-ocores.c 
 b/drivers/i2c/busses/i2c-ocores.c
 index bffd550..0b71dc6 100644
 --- a/drivers/i2c/busses/i2c-ocores.c
 +++ b/drivers/i2c/busses/i2c-ocores.c
 @@ -267,7 +267,8 @@ static int __devinit ocores_i2c_probe(struct 
 platform_device *pdev)
  {
  struct ocores_i2c *i2c;
  struct ocores_i2c_platform_data *pdata;
 -struct resource *res, *res2;
 +struct resource *res;
 +int irq;
  int ret;
  int i;
  
 @@ -275,8 +276,8 @@ static int __devinit ocores_i2c_probe(struct 
 platform_device *pdev)
  if (!res)
  return -ENODEV;
  
 -res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 -if (!res2)
 +irq = platform_get_irq(pdev, 0);
 +if (irq  0)
  return -ENODEV;
 
 Why not pass the error code you got?

I believe that should actually be a check for 'irq = 0' as it returns
NO_IRQ on error.

  
  i2c = devm_kzalloc(pdev-dev, sizeof(*i2c), GFP_KERNEL);
 @@ -313,7 +314,7 @@ static int __devinit ocores_i2c_probe(struct 
 platform_device *pdev)
  ocores_init(i2c);
  
  init_waitqueue_head(i2c-wait);
 -ret = devm_request_irq(pdev-dev, res2-start, ocores_isr, 0,
 +ret = devm_request_irq(pdev-dev, irq, ocores_isr, 0,
 pdev-name, i2c);
  if (ret) {
  dev_err(pdev-dev, Cannot claim IRQ\n);
 
 Rest looks good.
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: busses: i2c-ocores: Fix documentation filename

[Rob Herring]

On 09/23/2012 08:30 AM, Maxin B. John wrote:
 Fixes the wrong filename.
 
 Signed-off-by: Maxin B. John maxin.j...@gmail.com
 ---

Acked-by: Rob Herring rob.herr...@calxeda.com

 diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
 index bffd550..e39c122 100644
 --- a/drivers/i2c/busses/i2c-ocores.c
 +++ b/drivers/i2c/busses/i2c-ocores.c
 @@ -11,7 +11,7 @@
  
  /*
   * This driver can be used from the device tree, see
 - * Documentation/devicetree/bindings/i2c/ocore-i2c.txt
 + * Documentation/devicetree/bindings/i2c/i2c-ocores.txt
   */
  #include linux/kernel.h
  #include linux/module.h
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] i2c: nomadik: Add Device Tree support to the Nomadik I2C driver

[Rob Herring]

On 09/03/2012 09:35 AM, Linus Walleij wrote:
 On Mon, Sep 3, 2012 at 4:33 PM, Stephen Warren swar...@nvidia.com wrote:
 On 09/03/2012 05:58 AM, Linus Walleij wrote:
 On Mon, Sep 3, 2012 at 1:32 PM, Lee Jones lee.jo...@linaro.org wrote:

 No, this is wrong. Platform data should not override DT.

 If DT is enabled and passed, it should have highest priority.

 No, that's wrong. If platform data is specified, it overrides DT, so
 that if the DT needs any fixup, it can be provided using platform data.
 
 Thanks Stephen, now there are two of us saying this, Lee please
 follow this design pattern.
 
 (Unless Rob/Grant start shouting counter-orders...)

Ideally, you only use DT or platform_data and you override DT with a new
DTB. Hopefully we can ultimately remove platform_data or all but parts
that can't be described in DT (i.e. function callouts).

But if you are handling both, then I agree that platform_data should
override DT.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 13/15] i2c: iop3xx: clean-up trailing whitespace

[Rob Herring]

From: Rob Herring rob.herr...@calxeda.com

Remove a bunch of trailing whitespace. No functional changes.

Signed-off-by: Rob Herring rob.herr...@calxeda.com
Cc: Jean Delvare (PC drivers, core) kh...@linux-fr.org
Cc: Ben Dooks (embedded platforms) ben-li...@fluff.org
Cc: Wolfram Sang (embedded platforms) w.s...@pengutronix.de
Cc: linux-i2c@vger.kernel.org
---
 drivers/i2c/busses/i2c-iop3xx.c |  112 +++
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/drivers/i2c/busses/i2c-iop3xx.c b/drivers/i2c/busses/i2c-iop3xx.c
index 93f147a..567d873 100644
--- a/drivers/i2c/busses/i2c-iop3xx.c
+++ b/drivers/i2c/busses/i2c-iop3xx.c
@@ -4,13 +4,13 @@
 /* Copyright (C) 2003 Peter Milne, D-TACQ Solutions Ltd
  *Peter dot Milne at D hyphen TACQ dot com
  *
- * With acknowledgements to i2c-algo-ibm_ocp.c by 
+ * With acknowledgements to i2c-algo-ibm_ocp.c by
  * Ian DaSilva, MontaVista Software, Inc. idasi...@mvista.com
  *
  * And i2c-algo-pcf.c, which was created by Simon G. Vogl and Hans Berglund:
  *
  * Copyright (C) 1995-1997 Simon G. Vogl, 1998-2000 Hans Berglund
- *  
+ *
  * And which acknowledged Kyösti Mälkki kmal...@cc.hut.fi,
  * Frodo Looijaard fro...@dds.nl, Martin Baileymbai...@littlefeet-inc.com
  *
@@ -45,8 +45,8 @@
 /* global unit counter */
 static int i2c_id;
 
-static inline unsigned char 
-iic_cook_addr(struct i2c_msg *msg) 
+static inline unsigned char
+iic_cook_addr(struct i2c_msg *msg)
 {
unsigned char addr;
 
@@ -55,24 +55,24 @@ iic_cook_addr(struct i2c_msg *msg)
if (msg-flags  I2C_M_RD)
addr |= 1;
 
-   return addr;   
+   return addr;
 }
 
-static void 
+static void
 iop3xx_i2c_reset(struct i2c_algo_iop3xx_data *iop3xx_adap)
 {
/* Follows devman 9.3 */
__raw_writel(IOP3XX_ICR_UNIT_RESET, iop3xx_adap-ioaddr + CR_OFFSET);
__raw_writel(IOP3XX_ISR_CLEARBITS, iop3xx_adap-ioaddr + SR_OFFSET);
__raw_writel(0, iop3xx_adap-ioaddr + CR_OFFSET);
-} 
+}
 
-static void 
+static void
 iop3xx_i2c_enable(struct i2c_algo_iop3xx_data *iop3xx_adap)
 {
u32 cr = IOP3XX_ICR_GCD | IOP3XX_ICR_SCLEN | IOP3XX_ICR_UE;
 
-   /* 
+   /*
 * Every time unit enable is asserted, GPOD needs to be cleared
 * on IOP3XX to avoid data corruption on the bus.
 */
@@ -86,7 +86,7 @@ iop3xx_i2c_enable(struct i2c_algo_iop3xx_data *iop3xx_adap)
}
 #endif
/* NB SR bits not same position as CR IE bits :-( */
-   iop3xx_adap-SR_enabled = 
+   iop3xx_adap-SR_enabled =
IOP3XX_ISR_ALD | IOP3XX_ISR_BERRD |
IOP3XX_ISR_RXFULL | IOP3XX_ISR_TXEMPTY;
 
@@ -96,23 +96,23 @@ iop3xx_i2c_enable(struct i2c_algo_iop3xx_data *iop3xx_adap)
__raw_writel(cr, iop3xx_adap-ioaddr + CR_OFFSET);
 }
 
-static void 
+static void
 iop3xx_i2c_transaction_cleanup(struct i2c_algo_iop3xx_data *iop3xx_adap)
 {
unsigned long cr = __raw_readl(iop3xx_adap-ioaddr + CR_OFFSET);
-   
-   cr = ~(IOP3XX_ICR_MSTART | IOP3XX_ICR_TBYTE | 
+
+   cr = ~(IOP3XX_ICR_MSTART | IOP3XX_ICR_TBYTE |
IOP3XX_ICR_MSTOP | IOP3XX_ICR_SCLEN);
 
__raw_writel(cr, iop3xx_adap-ioaddr + CR_OFFSET);
 }
 
-/* 
- * NB: the handler has to clear the source of the interrupt! 
+/*
+ * NB: the handler has to clear the source of the interrupt!
  * Then it passes the SR flags of interest to BH via adap data
  */
-static irqreturn_t 
-iop3xx_i2c_irq_handler(int this_irq, void *dev_id) 
+static irqreturn_t
+iop3xx_i2c_irq_handler(int this_irq, void *dev_id)
 {
struct i2c_algo_iop3xx_data *iop3xx_adap = dev_id;
u32 sr = __raw_readl(iop3xx_adap-ioaddr + SR_OFFSET);
@@ -126,7 +126,7 @@ iop3xx_i2c_irq_handler(int this_irq, void *dev_id)
 }
 
 /* check all error conditions, clear them , report most important */
-static int 
+static int
 iop3xx_i2c_error(u32 sr)
 {
int rc = 0;
@@ -135,12 +135,12 @@ iop3xx_i2c_error(u32 sr)
if ( !rc ) rc = -I2C_ERR_BERR;
}
if ((sr  IOP3XX_ISR_ALD)) {
-   if ( !rc ) rc = -I2C_ERR_ALD;   
+   if ( !rc ) rc = -I2C_ERR_ALD;
}
-   return rc;  
+   return rc;
 }
 
-static inline u32 
+static inline u32
 iop3xx_i2c_get_srstat(struct i2c_algo_iop3xx_data *iop3xx_adap)
 {
unsigned long flags;
@@ -161,8 +161,8 @@ iop3xx_i2c_get_srstat(struct i2c_algo_iop3xx_data 
*iop3xx_adap)
 typedef int (* compare_func)(unsigned test, unsigned mask);
 /* returns 1 on correct comparison */
 
-static int 
-iop3xx_i2c_wait_event(struct i2c_algo_iop3xx_data *iop3xx_adap, 
+static int
+iop3xx_i2c_wait_event(struct i2c_algo_iop3xx_data *iop3xx_adap,
  unsigned flags, unsigned* status,
  compare_func compare)
 {
@@ -192,47 +192,47 @@ iop3xx_i2c_wait_event(struct i2c_algo_iop3xx_data 
*iop3xx_adap,
 }
 
 /*
- * Concrete compare_funcs 
+ * Concrete compare_funcs
  */
-static

[PATCH 14/15] i2c: iop3xx: use standard gpiolib functions

[Rob Herring]

From: Rob Herring rob.herr...@calxeda.com

Instead of using the custom iop3xx gpio functions, use the gpiolib
variants. This should be functionally the same since the gpiolib
just calls the iop3xx gpio functions. This is needed in preparation
of removing iop3xx mach/io.h headers.

Signed-off-by: Rob Herring rob.herr...@calxeda.com
Cc: Jean Delvare (PC drivers, core) kh...@linux-fr.org
Cc: Ben Dooks (embedded platforms) ben-li...@fluff.org
Cc: Wolfram Sang (embedded platforms) w.s...@pengutronix.de
Cc: linux-i2c@vger.kernel.org
---
 drivers/i2c/busses/i2c-iop3xx.c |9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-iop3xx.c b/drivers/i2c/busses/i2c-iop3xx.c
index 567d873..2f99613 100644
--- a/drivers/i2c/busses/i2c-iop3xx.c
+++ b/drivers/i2c/busses/i2c-iop3xx.c
@@ -39,6 +39,7 @@
 #include linux/platform_device.h
 #include linux/i2c.h
 #include linux/io.h
+#include linux/gpio.h
 
 #include i2c-iop3xx.h
 
@@ -78,11 +79,11 @@ iop3xx_i2c_enable(struct i2c_algo_iop3xx_data *iop3xx_adap)
 */
 #if defined(CONFIG_ARCH_IOP32X) || defined(CONFIG_ARCH_IOP33X)
if (iop3xx_adap-id == 0) {
-   gpio_line_set(IOP3XX_GPIO_LINE(7), GPIO_LOW);
-   gpio_line_set(IOP3XX_GPIO_LINE(6), GPIO_LOW);
+   gpio_set_value(7, 0);
+   gpio_set_value(6, 0);
} else {
-   gpio_line_set(IOP3XX_GPIO_LINE(5), GPIO_LOW);
-   gpio_line_set(IOP3XX_GPIO_LINE(4), GPIO_LOW);
+   gpio_set_value(5, 0);
+   gpio_set_value(4, 0);
}
 #endif
/* NB SR bits not same position as CR IE bits :-( */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] ARM: Kirkwood: Add interrupt controller support for DT boards

[Rob Herring]

On 06/10/2012 05:31 AM, Andrew Lunn wrote:
 Signed-off-by: Andrew Lunn and...@lunn.ch
 ---
  .../devicetree/bindings/arm/mrvl/intc.txt  |   20 ++
  arch/arm/boot/dts/kirkwood.dtsi|9 
  arch/arm/mach-kirkwood/board-dt.c  |   22 
 +++-
  3 files changed, 50 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/devicetree/bindings/arm/mrvl/intc.txt 
 b/Documentation/devicetree/bindings/arm/mrvl/intc.txt
 index 80b9a94..612536e 100644
 --- a/Documentation/devicetree/bindings/arm/mrvl/intc.txt
 +++ b/Documentation/devicetree/bindings/arm/mrvl/intc.txt
 @@ -38,3 +38,23 @@ Example:
   reg-names = mux status, mux mask;
   mrvl,intc-nr-irqs = 2;
   };
 +
 +* Marvell Orion Interrupt controller
 +
 +Required properties
 +- compatible :  Should be marvell,orion-intc
 +- #interrupt-cells: Specifies the number of cells needed to encode an
 +  interrupt source. Supported value is 1
 +- interrupt-controller : So that its clear its an interrupt controller.
 +Optional properties
 +- reg : Not used yet, but will contain the interrupt mask address
 +
 +Example:
 +
 + intc: interrupt-controller {
 + compatible = marvell,orion-intc, marvell,intc;
 + interrupt-controller;
 + #interrupt-cells = 1;
 +reg = 0xfed20204 0x04,
 +   0xfed20214 0x04;
 +};
 diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
 index 926528b..8eab7c4 100644
 --- a/arch/arm/boot/dts/kirkwood.dtsi
 +++ b/arch/arm/boot/dts/kirkwood.dtsi
 @@ -2,6 +2,15 @@
  
  / {
   compatible = mrvl,kirkwood;
 + interrupt-parent = intc;
 +
 + intc: interrupt-controller {
 + compatible = marvell,orion-intc, marvell,intc;
 + interrupt-controller;
 + #interrupt-cells = 1;
 +reg = 0xfed20204 0x04,
 +   0xfed20214 0x04;
 +};
  
   ocp@f100 {
   compatible = simple-bus;
 diff --git a/arch/arm/mach-kirkwood/board-dt.c 
 b/arch/arm/mach-kirkwood/board-dt.c
 index edc3f8a..fa51586 100644
 --- a/arch/arm/mach-kirkwood/board-dt.c
 +++ b/arch/arm/mach-kirkwood/board-dt.c
 @@ -14,6 +14,7 @@
  #include linux/init.h
  #include linux/of.h
  #include linux/of_platform.h
 +#include linux/of_irq.h
  #include linux/kexec.h
  #include asm/mach/arch.h
  #include asm/mach/map.h
 @@ -80,11 +81,30 @@ static const char *kirkwood_dt_board_compat[] = {
   NULL
  };
  
 +static int __init kirkwood_add_irq_domain(struct device_node *np,
 +   struct device_node *interrupt_parent)
 +{
 + kirkwood_init_irq();
 + irq_domain_add_legacy(np, 64, 0, 0, irq_domain_simple_ops, NULL);

The irqdomain should really be integrated into the interrupt controller
itself rather than bolted on top and doesn't need to be DT only. In the
process, use irq_data.hwirq rather than fixed Linux virq to hwirq
conversion.

Once all interrupts are bound using DT, a linear domain should be used.
But that may take some time.

Rob

 + return 0;
 +}rob.herr...@calxeda.com
 +
 +static const struct of_device_id kirkwood_irq_match[] = {
 + { .compatible = marvell,orion-intc,
 +   .data = kirkwood_add_irq_domain, },
 + {},
 +};
 +
 +static void __init kirkwood_dt_init_irq(void)
 +{
 + of_irq_init(kirkwood_irq_match);
 +}
 +
  DT_MACHINE_START(KIRKWOOD_DT, Marvell Kirkwood (Flattened Device Tree))
   /* Maintainer: Jason Cooper ja...@lakedaemon.net */
   .map_io = kirkwood_map_io,
   .init_early = kirkwood_init_early,
 - .init_irq   = kirkwood_init_irq,
 + .init_irq   = kirkwood_dt_init_irq,
   .timer  = kirkwood_timer,
   .init_machine   = kirkwood_dt_init,
   .restart= kirkwood_restart,

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] ARM: Kirkwood: Add DT support for GPIO controllers

[Rob Herring]

On 06/10/2012 05:32 AM, Andrew Lunn wrote:
 The GPIO controllers can now be described in DT. Origionally GPIO
 controllers were instantiated during IRQ setup. The origional none-DT
 code has been split out, and is only called if no DT GPIO controllers
 are found.
 
 Signed-off-by: Andrew Lunn and...@lunn.ch
 ---
  .../devicetree/bindings/gpio/mrvl-gpio.txt |   25 +++
  arch/arm/boot/dts/kirkwood.dtsi|   20 ++
  arch/arm/mach-kirkwood/irq.c   |   20 --
  arch/arm/plat-orion/gpio.c |   68 
 +++-
  arch/arm/plat-orion/include/plat/gpio.h|2 +
  5 files changed, 126 insertions(+), 9 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt 
 b/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
 index 05428f3..d94ebc1 100644
 --- a/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
 +++ b/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
 @@ -27,3 +27,28 @@ Example:
   interrupt-controller;
   #interrupt-cells = 1;
};
 +
 +* Marvell Orion GPIO Controller
 +
 +Required properties:
 +- compatible : Should be marvell,orion-gpio
 +- reg: Address and length of the register set for controller.
 +- gpio-controller: So we know this is a gpio controller.
 +- gpio-base  : Number of first gpio pin.
 +- ngpio  : How many gpios this controller has.
 +- secondary-irq-base : IRQ number base
 +
 +Optional properties:
 +- mask-offset: For SMP Orions, offset for Nth CPU
 +
 +Example:
 +
 + gpio0: gpio@10100 {
 + compatible = marvell,orion-gpio;
 + #gpio-cells = 2;
 + gpio-controller;
 + reg = 0x10100 0x40;
 + gpio-base = 0;
 + ngpio = 32;
 + secondary-irq-base = 64;

This and gpio-base are wrong. The DT should describe h/w and these are
Linux gpio and irq numbers. You need to create an irqdomain for the gpio
controller.

Rob

 + };
 diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
 index 3091c01..6de66dc 100644
 --- a/arch/arm/boot/dts/kirkwood.dtsi
 +++ b/arch/arm/boot/dts/kirkwood.dtsi
 @@ -18,6 +18,26 @@
   #address-cells = 1;
   #size-cells = 1;
  
 + gpio0: gpio@10100 {
 + compatible = marvell,orion-gpio;
 + #gpio-cells = 2;
 + gpio-controller;
 + reg = 0x10100 0x40;
 + gpio-base = 0;
 + ngpio = 32;
 + secondary-irq-base = 64;
 + };
 +
 + gpio1: gpio@10140 {
 + compatible = marvell,orion-gpio;
 + #gpio-cells = 2;
 + gpio-controller;
 + reg = 0x10140 0x40;
 + gpio-base = 32;
 + ngpio = 18;
 + secondary-irq-base = 96;
 + };
 +
   serial@12000 {
   compatible = ns16550a;
   reg = 0x12000 0x100;
 diff --git a/arch/arm/mach-kirkwood/irq.c b/arch/arm/mach-kirkwood/irq.c
 index c4c68e5..81340c2 100644
 --- a/arch/arm/mach-kirkwood/irq.c
 +++ b/arch/arm/mach-kirkwood/irq.c
 @@ -24,25 +24,33 @@ static void gpio_irq_handler(unsigned int irq, struct 
 irq_desc *desc)
   orion_gpio_irq_handler((irq - IRQ_KIRKWOOD_GPIO_LOW_0_7)  3);
  }
  
 -void __init kirkwood_init_irq(void)
 +static void __init kirkwood_init_gpio(void)
  {
 - orion_irq_init(0, (void __iomem *)(IRQ_VIRT_BASE + IRQ_MASK_LOW_OFF));
 - orion_irq_init(32, (void __iomem *)(IRQ_VIRT_BASE + IRQ_MASK_HIGH_OFF));
 -
   /*
* Initialize gpiolib for GPIOs 0-49.
*/
   orion_gpio_init(0, 32, GPIO_LOW_VIRT_BASE, 0,
   IRQ_KIRKWOOD_GPIO_START);
 + orion_gpio_init(32, 18, GPIO_HIGH_VIRT_BASE, 0,
 + IRQ_KIRKWOOD_GPIO_START + 32);
 +}
 +void __init kirkwood_init_irq(void)
 +{
 + orion_irq_init(0, (void __iomem *)(IRQ_VIRT_BASE + IRQ_MASK_LOW_OFF));
 + orion_irq_init(32, (void __iomem *)(IRQ_VIRT_BASE + IRQ_MASK_HIGH_OFF));
 +
   irq_set_chained_handler(IRQ_KIRKWOOD_GPIO_LOW_0_7, gpio_irq_handler);
   irq_set_chained_handler(IRQ_KIRKWOOD_GPIO_LOW_8_15, gpio_irq_handler);
   irq_set_chained_handler(IRQ_KIRKWOOD_GPIO_LOW_16_23, gpio_irq_handler);
   irq_set_chained_handler(IRQ_KIRKWOOD_GPIO_LOW_24_31, gpio_irq_handler);
  
 - orion_gpio_init(32, 18, GPIO_HIGH_VIRT_BASE, 0,
 - IRQ_KIRKWOOD_GPIO_START + 32);
   irq_set_chained_handler(IRQ_KIRKWOOD_GPIO_HIGH_0_7, gpio_irq_handler);
   irq_set_chained_handler(IRQ_KIRKWOOD_GPIO_HIGH_8_15, gpio_irq_handler);
   irq_set_chained_handler(IRQ_KIRKWOOD_GPIO_HIGH_16_23,
   gpio_irq_handler);
 +
 + /* Try initializing the GPIO 

Re: [PATCH V3] i2c: Add generic I2C multiplexer using pinctrl API

[Rob Herring]

On 05/01/2012 12:23 PM, Stephen Warren wrote:
 From: Stephen Warren swar...@nvidia.com
 
 This is useful for SoCs whose I2C module's signals can be routed to
 different sets of pins at run-time, using the pinctrl API.
 
  +-+  +-+
  | dev |  | dev |
 ++   +-+  +-+
 | SoC|  ||
 |   /|--++
 |   +---+   +--+ | child bus A, on first set of pins
 |   |I2C|---|Pinmux| |
 |   +---+   +--+ | child bus B, on second set of pins
 |   \|--+++
 ||  |||
 ++  +-+  +-+  +-+
 | dev |  | dev |  | dev |
 +-+  +-+  +-+


Such a pretty drawing, I think it deserves to be in the binding doc.

Otherwise, looks good to me:

Acked-by: Rob Herring rob.herr...@calxeda.com

Sorry for the delayed response.

Rob

 Signed-off-by: Stephen Warren swar...@nvidia.com
 Acked-by: Linus Walleij linus.wall...@linaro.org
 ---
 v3: Renamed pinctrl-i2cmux.c to i2c-mux-pinctrl.c to match recent changes
 to other I2C mux files.
 v2: Rebase onto David Daney's i2c-mux OF support, including slight binding
 changes. Add more error messages.
 
 squash pinctrl i2c mux
 ---
  .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt|   79 ++
  drivers/i2c/muxes/Kconfig  |   12 +
  drivers/i2c/muxes/Makefile |1 +
  drivers/i2c/muxes/i2c-mux-pinctrl.c|  279 
 
  include/linux/i2c-mux-pinctrl.h|   41 +++
  5 files changed, 412 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
  create mode 100644 drivers/i2c/muxes/i2c-mux-pinctrl.c
  create mode 100644 include/linux/i2c-mux-pinctrl.h
 
 diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
 new file mode 100644
 index 000..b10e268
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
 @@ -0,0 +1,79 @@
 +Pinctrl-based I2C Bus Mux
 +
 +This binding describes an I2C bus multiplexer that uses pin multiplexing to
 +route the I2C signals, and represents the pin multiplexing configuration
 +using the pinctrl device tree bindings.
 +
 +Required properties:
 +- compatible: i2c-mux-pinctrl
 +- i2c-parent: The phandle of the I2C bus that this multiplexer's master-side
 +  port is connected to.
 +
 +Also required are:
 +
 +* Standard pinctrl properties that specify the pin mux state for each child
 +  bus. See ../pinctrl/pinctrl-bindings.txt.
 +
 +* Standard I2C mux properties. See mux.txt in this directory.
 +
 +* I2C child bus nodes. See mux.txt in this directory.
 +
 +For each named state defined in the pinctrl-names property, an I2C child bus
 +will be created. I2C child bus numbers are assigned based on the index into
 +the pinctrl-names property.
 +
 +The only exception is that no bus will be created for a state named idle. 
 If
 +such a state is defined, it must be the last entry in pinctrl-names. For
 +example:
 +
 + pinctrl-names = ddc, pta, idle  -  ddc = bus 0, pta = bus 1
 + pinctrl-names = ddc, idle, pta  -  Invalid (idle not last)
 + pinctrl-names = idle, ddc, pta  -  Invalid (idle not last)
 +
 +Whenever an access is made to a device on a child bus, the relevant pinctrl
 +state will be programmed into hardware.
 +
 +If an idle state is defined, whenever an access is not being made to a device
 +on a child bus, the idle pinctrl state will be programmed into hardware.
 +
 +If an idle state is not defined, the most recently used pinctrl state will be
 +left programmed into hardware whenever no access is being made of a device on
 +a child bus.
 +
 +Example:
 +
 + i2cmux {
 + compatible = i2c-mux-pinctrl;
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + i2c-parent = i2c1;
 +
 + pinctrl-names = ddc, pta, idle;
 + pinctrl-0 = state_i2cmux_ddc;
 + pinctrl-1 = state_i2cmux_pta;
 + pinctrl-2 = state_i2cmux_idle;
 +
 + i2c@0 {
 + reg = 0;
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + eeprom {
 + compatible = eeprom;
 + reg = 0x50;
 + };
 + };
 +
 + i2c@1 {
 + reg = 1;
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + eeprom {
 + compatible = eeprom;
 + reg = 0x50

[PATCH 3/5] i2c: remove ixp2000 driver

[Rob Herring]

From: Rob Herring rob.herr...@calxeda.com

The platform is removed, so there are no users of this driver.

Signed-off-by: Rob Herring rob.herr...@calxeda.com
Cc: Jean Delvare (PC drivers, core) kh...@linux-fr.org
Cc: Ben Dooks (embedded platforms) ben-li...@fluff.org
Cc: Wolfram Sang (embedded platforms) w.s...@pengutronix.de
Cc: linux-i2c@vger.kernel.org
---
 drivers/i2c/busses/Kconfig   |   14 
 drivers/i2c/busses/Makefile  |1 -
 drivers/i2c/busses/i2c-ixp2000.c |  157 --
 3 files changed, 0 insertions(+), 172 deletions(-)
 delete mode 100644 drivers/i2c/busses/i2c-ixp2000.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d2c5095..188543d 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -445,20 +445,6 @@ config I2C_IOP3XX
  This driver can also be built as a module.  If so, the module
  will be called i2c-iop3xx.
 
-config I2C_IXP2000
-   tristate IXP2000 GPIO-Based I2C Interface (DEPRECATED)
-   depends on ARCH_IXP2000
-   select I2C_ALGOBIT
-   help
- Say Y here if you have an Intel IXP2000 (2400, 2800, 2850) based
- system and are using GPIO lines for an I2C bus.
-
- This support is also available as a module. If so, the module
- will be called i2c-ixp2000.
-
- This driver is deprecated and will be dropped soon. Use i2c-gpio
- instead.
-
 config I2C_MPC
tristate MPC107/824x/85xx/512x/52xx/83xx/86xx
depends on PPC
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 569567b..ce3c2be 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -44,7 +44,6 @@ obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
 obj-$(CONFIG_I2C_IMX)  += i2c-imx.o
 obj-$(CONFIG_I2C_INTEL_MID)+= i2c-intel-mid.o
 obj-$(CONFIG_I2C_IOP3XX)   += i2c-iop3xx.o
-obj-$(CONFIG_I2C_IXP2000)  += i2c-ixp2000.o
 obj-$(CONFIG_I2C_MPC)  += i2c-mpc.o
 obj-$(CONFIG_I2C_MV64XXX)  += i2c-mv64xxx.o
 obj-$(CONFIG_I2C_MXS)  += i2c-mxs.o
diff --git a/drivers/i2c/busses/i2c-ixp2000.c b/drivers/i2c/busses/i2c-ixp2000.c
deleted file mode 100644
index 5d263f90..000
--- a/drivers/i2c/busses/i2c-ixp2000.c
+++ /dev/null
@@ -1,157 +0,0 @@
-/*
- * drivers/i2c/busses/i2c-ixp2000.c
- *
- * I2C adapter for IXP2000 systems using GPIOs for I2C bus
- *
- * Author: Deepak Saxena dsax...@plexity.net
- * Based on IXDP2400 code by: Naeem M. Afzal naeem.m.af...@intel.com
- * Made generic by: Jeff Daly jeffrey.d...@intel.com
- *
- * Copyright (c) 2003-2004 MontaVista Software Inc.
- *
- * This file is licensed under  the terms of the GNU General Public 
- * License version 2. This program is licensed as is without any 
- * warranty of any kind, whether express or implied.
- *
- * From Jeff Daly:
- *
- * I2C adapter driver for Intel IXDP2xxx platforms. This should work for any
- * IXP2000 platform if it uses the HW GPIO in the same manner.  Basically, 
- * SDA and SCL GPIOs have external pullups.  Setting the respective GPIO to 
- * an input will make the signal a '1' via the pullup.  Setting them to 
- * outputs will pull them down. 
- *
- * The GPIOs are open drain signals and are used as configuration strap inputs
- * during power-up so there's generally a buffer on the board that needs to be 
- * 'enabled' to drive the GPIOs.
- */
-
-#include linux/kernel.h
-#include linux/init.h
-#include linux/platform_device.h
-#include linux/module.h
-#include linux/i2c.h
-#include linux/i2c-algo-bit.h
-#include linux/slab.h
-
-#include mach/hardware.h /* Pick up IXP2000-specific bits */
-#include mach/gpio-ixp2000.h
-
-static inline int ixp2000_scl_pin(void *data)
-{
-   return ((struct ixp2000_i2c_pins*)data)-scl_pin;
-}
-
-static inline int ixp2000_sda_pin(void *data)
-{
-   return ((struct ixp2000_i2c_pins*)data)-sda_pin;
-}
-
-
-static void ixp2000_bit_setscl(void *data, int val)
-{
-   int i = 5000;
-
-   if (val) {
-   gpio_line_config(ixp2000_scl_pin(data), GPIO_IN);
-   while(!gpio_line_get(ixp2000_scl_pin(data))  i--);
-   } else {
-   gpio_line_config(ixp2000_scl_pin(data), GPIO_OUT);
-   }
-}
-
-static void ixp2000_bit_setsda(void *data, int val)
-{
-   if (val) {
-   gpio_line_config(ixp2000_sda_pin(data), GPIO_IN);
-   } else {
-   gpio_line_config(ixp2000_sda_pin(data), GPIO_OUT);
-   }
-}
-
-static int ixp2000_bit_getscl(void *data)
-{
-   return gpio_line_get(ixp2000_scl_pin(data));
-}
-
-static int ixp2000_bit_getsda(void *data)
-{
-   return gpio_line_get(ixp2000_sda_pin(data));
-}
-
-struct ixp2000_i2c_data {
-   struct ixp2000_i2c_pins *gpio_pins;
-   struct i2c_adapter adapter;
-   struct i2c_algo_bit_data algo_data;
-};
-
-static int ixp2000_i2c_remove(struct platform_device *plat_dev)
-{
-   struct ixp2000_i2c_data *drv_data = platform_get_drvdata

Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.

[Rob Herring]

On 03/26/2012 07:27 PM, David Daney wrote:
 From: David Daney david.da...@cavium.com
 
 There are three parts to this:
 
 1) Remove the definitions of OCTEON_IRQ_TWSI and OCTEON_IRQ_TWSI2.
The interrupts are specified by the device tree and these hard
coded irq numbers block the used of the irq lines by the irq_domain
code.
 
 2) Remove platform device setup code from octeon-platform.c, it is
now unused.
 
 3) Convert i2c-octeon.c to use device tree.  Part of this includes
using the devm_* functions instead of the raw counterparts, thus
simplifying error handling.  No functionality is changed.

Shouldn't this patch go after converting the platform to DT?

 Signed-off-by: David Daney david.da...@cavium.com
 Cc: Jean Delvare (PC drivers, core) kh...@linux-fr.org
 Cc: Ben Dooks (embedded platforms) ben-li...@fluff.org
 Cc: Wolfram Sang (embedded platforms) w.s...@pengutronix.de
 Cc: linux-i2c@vger.kernel.org
 ---
 
 Should probably go via Ralf's linux-mips.org tree.
 
  arch/mips/cavium-octeon/octeon-irq.c  |2 -
  arch/mips/cavium-octeon/octeon-platform.c |   84 -
  arch/mips/include/asm/octeon/octeon.h |5 --
  drivers/i2c/busses/i2c-octeon.c   |   94 
 -
  4 files changed, 52 insertions(+), 133 deletions(-)

snip

  
 - if (i2c_data == NULL) {
 - dev_err(i2c-dev, no I2C frequency data\n);
 + /*
 +  * clock-rate is a legacy binding, the official binding is
 +  * clock-frequency.  Try the official one first and then
 +  * fall back if it doesn't exist.
 +  */
 + data = of_get_property(pdev-dev.of_node, clock-frequency, len);
 + if (!data || len != sizeof(*data))
 + data = of_get_property(pdev-dev.of_node, clock-rate, len);
 + if (data  len == sizeof(*data)) {
 + i2c-twsi_freq = be32_to_cpup(data);

Can't you use of_property_read_u32?

Does the legacy binding really exist as DT support is new?

Otherwise,

Acked-by: Rob Herring rob.herr...@calxeda.com

 + } else {
 + dev_err(i2c-dev,
 + no I2C 'clock-rate' or 'clock-frequency' property\n);
   result = -ENXIO;
 - goto fail_region;
 + goto out;
   }
  
 - i2c-twsi_phys = res_mem-start;
 - i2c-regsize = resource_size(res_mem);
 - i2c-twsi_freq = i2c_data-i2c_freq;
 - i2c-sys_freq = i2c_data-sys_freq;
 + i2c-sys_freq = octeon_get_io_clock_rate();
  
 - if (!request_mem_region(i2c-twsi_phys, i2c-regsize, res_mem-name)) {
 + if (!devm_request_mem_region(pdev-dev, i2c-twsi_phys, i2c-regsize,
 +   res_mem-name)) {
   dev_err(i2c-dev, request_mem_region failed\n);
 - goto fail_region;
 + goto out;
   }
 - i2c-twsi_base = ioremap(i2c-twsi_phys, i2c-regsize);
 + i2c-twsi_base = devm_ioremap(pdev-dev, i2c-twsi_phys, i2c-regsize);
  
   init_waitqueue_head(i2c-queue);
  
   i2c-irq = irq;
  
 - result = request_irq(i2c-irq, octeon_i2c_isr, 0, DRV_NAME, i2c);
 + result = devm_request_irq(pdev-dev, i2c-irq,
 +   octeon_i2c_isr, 0, DRV_NAME, i2c);
   if (result  0) {
   dev_err(i2c-dev, failed to attach interrupt\n);
 - goto fail_irq;
 + goto out;
   }
  
   result = octeon_i2c_initlowlevel(i2c);
   if (result) {
   dev_err(i2c-dev, init low level failed\n);
 - goto  fail_add;
 + goto  out;
   }
  
   result = octeon_i2c_setclock(i2c);
   if (result) {
   dev_err(i2c-dev, clock init failed\n);
 - goto  fail_add;
 + goto  out;
   }
  
   i2c-adap = octeon_i2c_ops;
   i2c-adap.dev.parent = pdev-dev;
 - i2c-adap.nr = pdev-id = 0 ? pdev-id : 0;
 + i2c-adap.dev.of_node = pdev-dev.of_node;
   i2c_set_adapdata(i2c-adap, i2c);
   platform_set_drvdata(pdev, i2c);
  
 - result = i2c_add_numbered_adapter(i2c-adap);
 + result = i2c_add_adapter(i2c-adap);
   if (result  0) {
   dev_err(i2c-dev, failed to add adapter\n);
   goto fail_add;
   }
 -
   dev_info(i2c-dev, version %s\n, DRV_VERSION);
  
 - return result;
 + of_i2c_register_devices(i2c-adap);
 +
 + return 0;
  
  fail_add:
   platform_set_drvdata(pdev, NULL);
 - free_irq(i2c-irq, i2c);
 -fail_irq:
 - iounmap(i2c-twsi_base);
 - release_mem_region(i2c-twsi_phys, i2c-regsize);
 -fail_region:
 - kfree(i2c);
  out:
   return result;
  };
 @@ -613,19 +619,24 @@ static int __devexit octeon_i2c_remove(struct 
 platform_device *pdev)
  
   i2c_del_adapter(i2c-adap);
   platform_set_drvdata(pdev, NULL);
 - free_irq(i2c-irq, i2c);
 - iounmap(i2c-twsi_base);
 - release_mem_region(i2c-twsi_phys, i2c-regsize);
 - kfree(i2c);
   return 0;
  };
  
 +static struct of_device_id

Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.

[Rob Herring]

On 03/26/2012 08:20 PM, Rob Herring wrote:
 On 03/26/2012 07:27 PM, David Daney wrote:
 From: David Daney david.da...@cavium.com

 There are three parts to this:

 1) Remove the definitions of OCTEON_IRQ_TWSI and OCTEON_IRQ_TWSI2.
The interrupts are specified by the device tree and these hard
coded irq numbers block the used of the irq lines by the irq_domain
code.

 2) Remove platform device setup code from octeon-platform.c, it is
now unused.

 3) Convert i2c-octeon.c to use device tree.  Part of this includes
using the devm_* functions instead of the raw counterparts, thus
simplifying error handling.  No functionality is changed.
 
 Shouldn't this patch go after converting the platform to DT?

Nevermind this comment...

 
 Signed-off-by: David Daney david.da...@cavium.com
 Cc: Jean Delvare (PC drivers, core) kh...@linux-fr.org
 Cc: Ben Dooks (embedded platforms) ben-li...@fluff.org
 Cc: Wolfram Sang (embedded platforms) w.s...@pengutronix.de
 Cc: linux-i2c@vger.kernel.org
 ---

 Should probably go via Ralf's linux-mips.org tree.

  arch/mips/cavium-octeon/octeon-irq.c  |2 -
  arch/mips/cavium-octeon/octeon-platform.c |   84 -
  arch/mips/include/asm/octeon/octeon.h |5 --
  drivers/i2c/busses/i2c-octeon.c   |   94 
 -
  4 files changed, 52 insertions(+), 133 deletions(-)
 
 snip
 
  
 -if (i2c_data == NULL) {
 -dev_err(i2c-dev, no I2C frequency data\n);
 +/*
 + * clock-rate is a legacy binding, the official binding is
 + * clock-frequency.  Try the official one first and then
 + * fall back if it doesn't exist.
 + */
 +data = of_get_property(pdev-dev.of_node, clock-frequency, len);
 +if (!data || len != sizeof(*data))
 +data = of_get_property(pdev-dev.of_node, clock-rate, len);
 +if (data  len == sizeof(*data)) {
 +i2c-twsi_freq = be32_to_cpup(data);
 
 Can't you use of_property_read_u32?
 
 Does the legacy binding really exist as DT support is new?
 
 Otherwise,
 
 Acked-by: Rob Herring rob.herr...@calxeda.com
 
 +} else {
 +dev_err(i2c-dev,
 +no I2C 'clock-rate' or 'clock-frequency' property\n);
  result = -ENXIO;
 -goto fail_region;
 +goto out;
  }
  
 -i2c-twsi_phys = res_mem-start;
 -i2c-regsize = resource_size(res_mem);
 -i2c-twsi_freq = i2c_data-i2c_freq;
 -i2c-sys_freq = i2c_data-sys_freq;
 +i2c-sys_freq = octeon_get_io_clock_rate();
  
 -if (!request_mem_region(i2c-twsi_phys, i2c-regsize, res_mem-name)) {
 +if (!devm_request_mem_region(pdev-dev, i2c-twsi_phys, i2c-regsize,
 +  res_mem-name)) {
  dev_err(i2c-dev, request_mem_region failed\n);
 -goto fail_region;
 +goto out;
  }
 -i2c-twsi_base = ioremap(i2c-twsi_phys, i2c-regsize);
 +i2c-twsi_base = devm_ioremap(pdev-dev, i2c-twsi_phys, i2c-regsize);
  
  init_waitqueue_head(i2c-queue);
  
  i2c-irq = irq;
  
 -result = request_irq(i2c-irq, octeon_i2c_isr, 0, DRV_NAME, i2c);
 +result = devm_request_irq(pdev-dev, i2c-irq,
 +  octeon_i2c_isr, 0, DRV_NAME, i2c);
  if (result  0) {
  dev_err(i2c-dev, failed to attach interrupt\n);
 -goto fail_irq;
 +goto out;
  }
  
  result = octeon_i2c_initlowlevel(i2c);
  if (result) {
  dev_err(i2c-dev, init low level failed\n);
 -goto  fail_add;
 +goto  out;
  }
  
  result = octeon_i2c_setclock(i2c);
  if (result) {
  dev_err(i2c-dev, clock init failed\n);
 -goto  fail_add;
 +goto  out;
  }
  
  i2c-adap = octeon_i2c_ops;
  i2c-adap.dev.parent = pdev-dev;
 -i2c-adap.nr = pdev-id = 0 ? pdev-id : 0;
 +i2c-adap.dev.of_node = pdev-dev.of_node;
  i2c_set_adapdata(i2c-adap, i2c);
  platform_set_drvdata(pdev, i2c);
  
 -result = i2c_add_numbered_adapter(i2c-adap);
 +result = i2c_add_adapter(i2c-adap);
  if (result  0) {
  dev_err(i2c-dev, failed to add adapter\n);
  goto fail_add;
  }
 -
  dev_info(i2c-dev, version %s\n, DRV_VERSION);
  
 -return result;
 +of_i2c_register_devices(i2c-adap);
 +
 +return 0;
  
  fail_add:
  platform_set_drvdata(pdev, NULL);
 -free_irq(i2c-irq, i2c);
 -fail_irq:
 -iounmap(i2c-twsi_base);
 -release_mem_region(i2c-twsi_phys, i2c-regsize);
 -fail_region:
 -kfree(i2c);
  out:
  return result;
  };
 @@ -613,19 +619,24 @@ static int __devexit octeon_i2c_remove(struct 
 platform_device *pdev)
  
  i2c_del_adapter(i2c-adap);
  platform_set_drvdata(pdev, NULL);
 -free_irq(i2c-irq, i2c);
 -iounmap(i2c-twsi_base);
 -release_mem_region(i2c-twsi_phys, i2c-regsize);
 -kfree(i2c);
  return 0;
  };
  
 +static struct of_device_id

Re: [PATCH 1/5 v3] i2c/gpio: add DT support

[Rob Herring]

On 03/08/2012 02:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
 Cc: Nicolas Ferre nicolas.fe...@atmel.com
 Cc: linux-i2c@vger.kernel.org
 Cc: devicetree-disc...@lists.ozlabs.org
 ---
 v3:
 
   update i2c binding (Rob comments)
 
   Jean can I have a ack to apply it?
 
 Best Regards,
 J.

For this patch:

Acked-by: Rob Herring rob.herr...@calxeda.com


  .../devicetree/bindings/gpio/gpio_i2c.txt  |   32 +++
  drivers/i2c/busses/i2c-gpio.c  |   94 +++
  2 files changed, 106 insertions(+), 20 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt
 
 diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt 
 b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
 new file mode 100644
 index 000..4f8ec94
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
 @@ -0,0 +1,32 @@
 +Device-Tree bindings for i2c gpio driver
 +
 +Required properties:
 + - compatible = i2c-gpio;
 + - gpios: sda and scl gpio
 +
 +
 +Optional properties:
 + - i2c-gpio,sda-open-drain: sda as open drain
 + - i2c-gpio,scl-open-drain: scl as open drain
 + - i2c-gpio,scl-output-only: scl as output only
 + - i2c-gpio,delay-us: delay between GPIO operations (may depend on each 
 platform)
 + - i2c-gpio,timeout-ms: timeout to get data
 +
 +Example nodes:
 +
 +i2c@0 {
 + compatible = i2c-gpio;
 + gpios = pioA 23 0 /* sda */
 +  pioA 24 0 /* scl */
 + ;
 + i2c-gpio,sda-open-drain;
 + i2c-gpio,scl-open-drain;
 + i2c-gpio,delay-us = 2;/* ~100 kHz */
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + rv3029c2@56 {
 + compatible = rv3029c2;
 + reg = 0x56;
 + };
 +};
 diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
 index a651779..98ae85b 100644
 --- a/drivers/i2c/busses/i2c-gpio.c
 +++ b/drivers/i2c/busses/i2c-gpio.c
 @@ -14,8 +14,15 @@
  #include linux/module.h
  #include linux/slab.h
  #include linux/platform_device.h
 +#include linux/gpio.h
 +#include linux/of_gpio.h
 +#include linux/of_i2c.h
  
 -#include asm/gpio.h
 +struct i2c_gpio_private_data {
 + struct i2c_adapter adap;
 + struct i2c_algo_bit_data bit_data;
 + struct i2c_gpio_platform_data pdata;
 +};
  
  /* Toggle SDA by changing the direction of the pin */
  static void i2c_gpio_setsda_dir(void *data, int state)
 @@ -78,24 +85,62 @@ static int i2c_gpio_getscl(void *data)
   return gpio_get_value(pdata-scl_pin);
  }
  
 +static int __devinit of_i2c_gpio_probe(struct device_node *np,
 +  struct i2c_gpio_platform_data *pdata)
 +{
 + u32 reg;
 +
 + if (of_gpio_count(np)  2)
 + return -ENODEV;
 +
 + pdata-sda_pin = of_get_gpio(np, 0);
 + pdata-scl_pin = of_get_gpio(np, 1);
 +
 + if (pdata-sda_pin  0 || pdata-scl_pin  0) {
 + pr_err(%s: invalid GPIO pins, sda=%d/scl=%d\n,
 +np-full_name, pdata-sda_pin, pdata-scl_pin);
 + return -ENODEV;
 + }
 +
 + of_property_read_u32(np, i2c-gpio,delay-us, pdata-udelay);
 +
 + if (of_property_read_u32(np, i2c-gpio,timeout-ms, reg))
 + pdata-timeout = msecs_to_jiffies(reg);
 +
 + pdata-sda_is_open_drain =
 + !!of_property_read_bool(np, i2c-gpio,sda-open-drain);
 + pdata-scl_is_open_drain =
 + !!of_property_read_bool(np, i2c-gpio,scl-open-drain);
 + pdata-scl_is_output_only =
 + !!of_property_read_bool(np, i2c-gpio,scl-output-only);
 +
 + return 0;
 +}
 +
  static int __devinit i2c_gpio_probe(struct platform_device *pdev)
  {
 + struct i2c_gpio_private_data *priv;
   struct i2c_gpio_platform_data *pdata;
   struct i2c_algo_bit_data *bit_data;
   struct i2c_adapter *adap;
   int ret;
  
 - pdata = pdev-dev.platform_data;
 - if (!pdata)
 - return -ENXIO;
 + priv = devm_kzalloc(pdev-dev, sizeof(*priv), GFP_KERNEL);
 + if (!priv)
 + return -ENOMEM;
 + adap = priv-adap;
 + bit_data = priv-bit_data;
 + pdata = priv-pdata;
  
 - ret = -ENOMEM;
 - adap = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
 - if (!adap)
 - goto err_alloc_adap;
 - bit_data = kzalloc(sizeof(struct i2c_algo_bit_data), GFP_KERNEL);
 - if (!bit_data)
 - goto err_alloc_bit_data;
 + if (pdev-dev.of_node) {
 + ret = of_i2c_gpio_probe(pdev-dev.of_node, pdata);
 + if (ret)
 + return ret;
 + } else {
 + if (!pdev-dev.platform_data)
 + return -ENXIO;
 + memcpy(pdata, pdev-dev.platform_data, sizeof(*pdata));
 + }
  
   ret = gpio_request(pdata-sda_pin, sda);
   if (ret)
 @@ -143,6 +188,7 @@ static int __devinit i2c_gpio_probe(struct 
 platform_device *pdev

Re: [PATCH 1/5 v2] i2c/gpio: add DT support

[Rob Herring]

On 02/29/2012 08:20 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
 Cc: Nicolas Ferre nicolas.fe...@atmel.com
 Cc: linux-i2c@vger.kernel.org
 Cc: devicetree-disc...@lists.ozlabs.org
 ---
 v2:
 
   use devm_kzalloc
   use i2c-gpio
   use time name in the properties
 
 Best Regars,
 J.
  .../devicetree/bindings/gpio/gpio_i2c.txt  |   32 +++
  drivers/i2c/busses/i2c-gpio.c  |   95 +++
  2 files changed, 107 insertions(+), 20 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt
 
 diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt 
 b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
 new file mode 100644
 index 000..27e1b1a
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
 @@ -0,0 +1,32 @@
 +Device-Tree bindings for i2c gpio driver
 +
 +Required properties:
 + - compatible = i2c-gpio;
 + - gpios: sda and scl gpio
 +
 +
 +Optional properties:
 + - i2c-gpio,sda-open-drain: sda as open drain
 + - i2c-gpio,scl-open-drain: scl as open drain
 + - i2c-gpio,scl-output-only: scl as output only
 + - udelay: delay between GPIO operations (may depend on each platform)

i2c-gpio,delay-us

 + - i2c-algo-bit,timeout-milliseconds: timeout to get data

i2c-gpio,timeout-ms

 +
 +Example nodes:
 +
 +i2c-gpio@0 {

Should be i2c@0

 + compatible = i2c-gpio;
 + gpios = pioA 23 0 /* sda */
 +  pioA 24 0 /* scl */
 + ;
 + i2c-gpio,sda-open-drain;
 + i2c-gpio,scl-open-drain;
 + udelay = 2;   /* ~100 kHz */
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + rv3029c2@56 {
 + compatible = rv3029c2;
 + reg = 0x56;
 + };
 +};
 diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
 index a651779..71adba5 100644
 --- a/drivers/i2c/busses/i2c-gpio.c
 +++ b/drivers/i2c/busses/i2c-gpio.c
 @@ -14,8 +14,15 @@
  #include linux/module.h
  #include linux/slab.h
  #include linux/platform_device.h
 +#include linux/gpio.h
 +#include linux/of_gpio.h
 +#include linux/of_i2c.h
  
 -#include asm/gpio.h
 +struct i2c_gpio_private_data {
 + struct i2c_adapter adap;
 + struct i2c_algo_bit_data bit_data;
 + struct i2c_gpio_platform_data pdata;
 +};
  
  /* Toggle SDA by changing the direction of the pin */
  static void i2c_gpio_setsda_dir(void *data, int state)
 @@ -78,24 +85,63 @@ static int i2c_gpio_getscl(void *data)
   return gpio_get_value(pdata-scl_pin);
  }
  
 +static int __devinit of_i2c_gpio_probe(struct device_node *np,
 +  struct i2c_gpio_platform_data *pdata)
 +{
 + u32 reg;
 +
 + if (of_gpio_count(np)  2)
 + return -ENODEV;
 +
 + pdata-sda_pin = of_get_gpio(np, 0);
 + pdata-scl_pin = of_get_gpio(np, 1);
 +
 + if (pdata-sda_pin  0 || pdata-scl_pin  0) {
 + pr_err(%s: invalid GPIO pins, sda=%d/scl=%d\n,
 +np-full_name, pdata-sda_pin, pdata-scl_pin);
 + return -ENODEV;
 + }
 +
 + if (of_property_read_u32(np, udelay, reg))
 + pdata-udelay = reg;

Why not of_property_read_u32(np, udelay, pdata-udelay)?

Also, reg may not be initialized here.

 +
 + if (of_property_read_u32(np, i2c-algo-bit,timeout-milliseconds, reg))
 + pdata-timeout = msecs_to_jiffies(reg);
 +
 + pdata-sda_is_open_drain =
 + !!of_property_read_bool(np, i2c-gpio,sda-open-drain);
 + pdata-scl_is_open_drain =
 + !!of_property_read_bool(np, i2c-gpio,scl-open-drain);
 + pdata-scl_is_output_only =
 + !!of_property_read_bool(np, i2c-gpio,scl-output-only);
 +
 + return 0;
 +}
 +
  static int __devinit i2c_gpio_probe(struct platform_device *pdev)
  {
 + struct i2c_gpio_private_data *priv;
   struct i2c_gpio_platform_data *pdata;
   struct i2c_algo_bit_data *bit_data;
   struct i2c_adapter *adap;
   int ret;
  
 - pdata = pdev-dev.platform_data;
 - if (!pdata)
 - return -ENXIO;
 + priv = devm_kzalloc(pdev-dev, sizeof(*priv), GFP_KERNEL);
 + if (!priv)
 + return -ENOMEM;
 + adap = priv-adap;
 + bit_data = priv-bit_data;
 + pdata = priv-pdata;
  
 - ret = -ENOMEM;
 - adap = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
 - if (!adap)
 - goto err_alloc_adap;
 - bit_data = kzalloc(sizeof(struct i2c_algo_bit_data), GFP_KERNEL);
 - if (!bit_data)
 - goto err_alloc_bit_data;
 + if (pdev-dev.of_node) {
 + ret = of_i2c_gpio_probe(pdev-dev.of_node, pdata);
 + if (ret)
 + return ret;
 + } else {
 + if (!pdev-dev.platform_data)
 + return -ENXIO;
 + memcpy(pdata, pdev-dev.platform_data, sizeof(*pdata));
 + }
  
   ret = gpio_request(pdata-sda_pin, 

Re: [PATCH 1/5 v2] i2c/gpio: add DT support

[Rob Herring]

On 03/07/2012 11:22 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
 On 11:16 Wed 07 Mar , Rob Herring wrote:
 On 02/29/2012 08:20 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
 Cc: Nicolas Ferre nicolas.fe...@atmel.com
 Cc: linux-i2c@vger.kernel.org
 Cc: devicetree-disc...@lists.ozlabs.org
 ---
 v2:

 use devm_kzalloc
 use i2c-gpio
 use time name in the properties

 Best Regars,
 J.
  .../devicetree/bindings/gpio/gpio_i2c.txt  |   32 +++
  drivers/i2c/busses/i2c-gpio.c  |   95 
 +++
  2 files changed, 107 insertions(+), 20 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt

 diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt 
 b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
 new file mode 100644
 index 000..27e1b1a
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
 @@ -0,0 +1,32 @@
 +Device-Tree bindings for i2c gpio driver
 +
 +Required properties:
 +   - compatible = i2c-gpio;
 +   - gpios: sda and scl gpio
 +
 +
 +Optional properties:
 +   - i2c-gpio,sda-open-drain: sda as open drain
 +   - i2c-gpio,scl-open-drain: scl as open drain
 +   - i2c-gpio,scl-output-only: scl as output only
 +   - udelay: delay between GPIO operations (may depend on each platform)

 i2c-gpio,delay-us

 +   - i2c-algo-bit,timeout-milliseconds: timeout to get data

 i2c-gpio,timeout-ms
 it's algo-bit speficic no i2c-gpio that's why I use i2c-algo-bit

Don't bring Linux into the binding. I would have to look at the Linux
driver to understand your reasoning.


 +
 +Example nodes:
 +
 +i2c-gpio@0 {

 Should be i2c@0
 ok

 +   compatible = i2c-gpio;
 +   gpios = pioA 23 0 /* sda */
 +pioA 24 0 /* scl */
 +   ;
 +   i2c-gpio,sda-open-drain;
 +   i2c-gpio,scl-open-drain;
 +   udelay = 2;   /* ~100 kHz */
 +   #address-cells = 1;
 +   #size-cells = 0;
 +
 +   rv3029c2@56 {
 +   compatible = rv3029c2;
 +   reg = 0x56;
 +   };
 +};
 diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
 index a651779..71adba5 100644
 --- a/drivers/i2c/busses/i2c-gpio.c
 +++ b/drivers/i2c/busses/i2c-gpio.c
 @@ -14,8 +14,15 @@
  #include linux/module.h
  #include linux/slab.h
  #include linux/platform_device.h
 +#include linux/gpio.h
 +#include linux/of_gpio.h
 +#include linux/of_i2c.h
  
 -#include asm/gpio.h
 +struct i2c_gpio_private_data {
 +   struct i2c_adapter adap;
 +   struct i2c_algo_bit_data bit_data;
 +   struct i2c_gpio_platform_data pdata;
 +};
  
  /* Toggle SDA by changing the direction of the pin */
  static void i2c_gpio_setsda_dir(void *data, int state)
 @@ -78,24 +85,63 @@ static int i2c_gpio_getscl(void *data)
 return gpio_get_value(pdata-scl_pin);
  }
  
 +static int __devinit of_i2c_gpio_probe(struct device_node *np,
 +struct i2c_gpio_platform_data *pdata)
 +{
 +   u32 reg;
 +
 +   if (of_gpio_count(np)  2)
 +   return -ENODEV;
 +
 +   pdata-sda_pin = of_get_gpio(np, 0);
 +   pdata-scl_pin = of_get_gpio(np, 1);
 +
 +   if (pdata-sda_pin  0 || pdata-scl_pin  0) {
 +   pr_err(%s: invalid GPIO pins, sda=%d/scl=%d\n,
 +  np-full_name, pdata-sda_pin, pdata-scl_pin);
 +   return -ENODEV;
 +   }
 +
 +   if (of_property_read_u32(np, udelay, reg))
 +   pdata-udelay = reg;

 Why not of_property_read_u32(np, udelay, pdata-udelay)?

 Also, reg may not be initialized here.
 if udelay have any error we keep udelay as 0
 

Right, so there is no init issue with reg, but still you can just right
this and get rid of the if:

of_property_read_u32(np, udelay, pdata-udelay);

If there's an error, the value will be unchanged.

Rob

 Best Regards,
 J.

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PULL] designware i2c DT binding support

[Rob Herring]

Ben,

Please pull for 3.3. This missed 3.2 and I have rebased to the recent
plat/pci changes.

The following changes since commit caca6a03d365883564885f2c1da3e88dcf65d139:

  Linux 3.2-rc3 (2011-11-23 20:20:28 -0800)

are available in the git repository at:
  git://sources.calxeda.com/kernel/linux.git i2c-for-3.3

Rob Herring (1):
  i2c-designware: add OF binding support

 .../devicetree/bindings/i2c/i2c-designware.txt |   22

 drivers/i2c/busses/i2c-designware-platdrv.c|   12 ++
 2 files changed, 34 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt

Thanks,
Rob
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] i2c-designware: add OF binding support

[Rob Herring]

From: Rob Herring rob.herr...@calxeda.com

Add of_match_table and DT style i2c registration to designware i2c
driver.

Signed-off-by: Rob Herring rob.herr...@calxeda.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Ben Dooks ben-li...@fluff.org
Cc: linux-i2c@vger.kernel.org
---
Ben,

Please apply. I've dropped moving of i2c functions into i2c core and
reverted back to my original patch and addressed your's and Grant's
comments:

- use of_match_ptr()
- always call i2c_add_numbered_adapter and drop ifdef

Rob

 Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 ++
 drivers/i2c/busses/i2c-designware.c  |   12 +++
 2 files changed, 35 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt

diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
new file mode 100644
index 000..cbcb404
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
@@ -0,0 +1,23 @@
+* Synopsys DesignWare I2C
+
+Required properties :
+
+ - compatible : should be snps,designware-i2c
+ - reg : Offset and length of the register set for the device
+ - interrupts : IRQ where IRQ is the interrupt number.
+
+Recommended properties :
+
+ - clock-frequency : desired I2C bus clock frequency in Hz.
+
+Example :
+
+   i2c@f {
+   #address-cells = 1;
+   #size-cells = 0;
+   compatible = snps,designware-i2c;
+   reg = 0xf 0x1000;
+   interrupts = 11;
+   clock-frequency = 40;
+   };
+
diff --git a/drivers/i2c/busses/i2c-designware.c 
b/drivers/i2c/busses/i2c-designware.c
index b7a51c4..9047bfd 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -34,6 +34,7 @@
 #include linux/sched.h
 #include linux/err.h
 #include linux/interrupt.h
+#include linux/of_i2c.h
 #include linux/platform_device.h
 #include linux/io.h
 #include linux/slab.h
@@ -769,6 +770,7 @@ static int __devinit dw_i2c_probe(struct platform_device 
*pdev)
sizeof(adap-name));
adap-algo = i2c_dw_algo;
adap-dev.parent = pdev-dev;
+   adap-dev.of_node = pdev-dev.of_node;
 
adap-nr = pdev-id;
r = i2c_add_numbered_adapter(adap);
@@ -776,6 +778,7 @@ static int __devinit dw_i2c_probe(struct platform_device 
*pdev)
dev_err(pdev-dev, failure adding adapter\n);
goto err_free_irq;
}
+   of_i2c_register_devices(adap);
 
return 0;
 
@@ -819,6 +822,14 @@ static int __devexit dw_i2c_remove(struct platform_device 
*pdev)
return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id dw_i2c_of_match[] = {
+   { .compatible = snps,designware-i2c, },
+   {},
+};
+MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
+#endif
+
 /* work with hotplug and coldplug */
 MODULE_ALIAS(platform:i2c_designware);
 
@@ -827,6 +838,7 @@ static struct platform_driver dw_i2c_driver = {
.driver = {
.name   = i2c_designware,
.owner  = THIS_MODULE,
+   .of_match_table = of_match_ptr(dw_i2c_of_match),
},
 };
 
-- 
1.7.5.4

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH REPOST 2/2] i2c-imx: add device tree probe support

[Rob Herring]

Shawn,

On 09/08/2011 02:09 AM, Shawn Guo wrote:
 It adds device tree probe support for i2c-imx driver.
 
 Signed-off-by: Shawn Guo shawn@linaro.org
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: Darius Augulis augulis.dar...@gmail.com
 Cc: Ben Dooks ben-li...@fluff.org
 Acked-by: Grant Likely grant.lik...@secretlab.ca
 ---
  .../devicetree/bindings/i2c/fsl-imx-i2c.txt|   25 
 
  drivers/i2c/busses/i2c-imx.c   |   25 +++
  2 files changed, 44 insertions(+), 6 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/i2c/fsl-imx-i2c.txt
 
 diff --git a/Documentation/devicetree/bindings/i2c/fsl-imx-i2c.txt 
 b/Documentation/devicetree/bindings/i2c/fsl-imx-i2c.txt
 new file mode 100644
 index 000..f3cf43b
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/fsl-imx-i2c.txt
 @@ -0,0 +1,25 @@
 +* Freescale Inter IC (I2C) and High Speed Inter IC (HS-I2C) for i.MX
 +
 +Required properties:
 +- compatible : Should be fsl,chip-i2c
 +- reg : Should contain I2C/HS-I2C registers location and length
 +- interrupts : Should contain I2C/HS-I2C interrupt
 +
 +Optional properties:
 +- clock-frequency : Constains desired I2C/HS-I2C bus clock frequency in Hz.
 +  The absence of the propoerty indicates the default frequency 100 kHz.
 +
 +Examples:
 +
 +i2c@83fc4000 { /* I2C2 on i.MX51 */
 + compatible = fsl,imx51-i2c, fsl,imx1-i2c;
 + reg = 0x83fc4000 0x4000;
 + interrupts = 63;
 +};
 +
 +i2c@70038000 { /* HS-I2C on i.MX51 */

The mainline driver doesn't support the HS-I2C block does it?

The high speed and old i2c blocks on MX51 are basically different
blocks, so the compatible property should be different.

Rob

 + compatible = fsl,imx51-i2c, fsl,imx1-i2c;
 + reg = 0x70038000 0x4000;
 + interrupts = 64;
 + clock-frequency = 40;
 +};
 diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
 index 54d809e..58832e5 100644
 --- a/drivers/i2c/busses/i2c-imx.c
 +++ b/drivers/i2c/busses/i2c-imx.c
 @@ -48,6 +48,9 @@
  #include linux/platform_device.h
  #include linux/clk.h
  #include linux/slab.h
 +#include linux/of.h
 +#include linux/of_device.h
 +#include linux/of_i2c.h
  
  #include mach/irqs.h
  #include mach/hardware.h
 @@ -125,6 +128,11 @@ struct imx_i2c_struct {
   unsigned intifdr; /* IMX_I2C_IFDR */
  };
  
 +static const struct of_device_id i2c_imx_dt_ids[] = {
 + { .compatible = fsl,imx1-i2c, },
 + { /* sentinel */ }
 +};
 +
  /** Functions for IMX I2C adapter driver 
 ***
  
 ***/
  
 @@ -469,7 +477,7 @@ static int __init i2c_imx_probe(struct platform_device 
 *pdev)
   struct imxi2c_platform_data *pdata = pdev-dev.platform_data;
   void __iomem *base;
   resource_size_t res_size;
 - int irq;
 + int irq, bitrate;
   int ret;
  
   dev_dbg(pdev-dev, %s\n, __func__);
 @@ -512,6 +520,7 @@ static int __init i2c_imx_probe(struct platform_device 
 *pdev)
   i2c_imx-adapter.algo   = i2c_imx_algo;
   i2c_imx-adapter.dev.parent = pdev-dev;
   i2c_imx-adapter.nr = pdev-id;
 + i2c_imx-adapter.dev.of_node= pdev-dev.of_node;
   i2c_imx-irq= irq;
   i2c_imx-base   = base;
   i2c_imx-res= res;
 @@ -538,10 +547,12 @@ static int __init i2c_imx_probe(struct platform_device 
 *pdev)
   i2c_set_adapdata(i2c_imx-adapter, i2c_imx);
  
   /* Set up clock divider */
 - if (pdata  pdata-bitrate)
 - i2c_imx_set_clk(i2c_imx, pdata-bitrate);
 - else
 - i2c_imx_set_clk(i2c_imx, IMX_I2C_BIT_RATE);
 + bitrate = IMX_I2C_BIT_RATE;
 + ret = of_property_read_u32(pdev-dev.of_node,
 +clock-frequency, bitrate);
 + if (ret  0  pdata  pdata-bitrate)
 + bitrate = pdata-bitrate;
 + i2c_imx_set_clk(i2c_imx, bitrate);
  
   /* Set up chip registers to defaults */
   writeb(0, i2c_imx-base + IMX_I2C_I2CR);
 @@ -554,6 +565,8 @@ static int __init i2c_imx_probe(struct platform_device 
 *pdev)
   goto fail5;
   }
  
 + of_i2c_register_devices(i2c_imx-adapter);
 +
   /* Set up platform driver data */
   platform_set_drvdata(pdev, i2c_imx);
  
 @@ -584,7 +597,6 @@ fail1:
  static int __exit i2c_imx_remove(struct platform_device *pdev)
  {
   struct imx_i2c_struct *i2c_imx = platform_get_drvdata(pdev);
 - struct imxi2c_platform_data *pdata = pdev-dev.platform_data;
  
   /* remove adapter */
   dev_dbg(i2c_imx-adapter.dev, adapter removed\n);
 @@ -613,6 +625,7 @@ static struct platform_driver i2c_imx_driver = {
   .driver = {
   .name   = DRIVER_NAME,
   .owner  = THIS_MODULE,
 + .of_match_table = i2c_imx_dt_ids,
   }
  };
  

--
To unsubscribe from this list: send the 

Re: [PATCH 3/3] i2c-designware: add OF binding support

[Rob Herring]

Ben,

On 08/05/2011 04:24 PM, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com
 
 Add of_match_table and DT style i2c registration to designware i2c
 driver.
 
 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Acked-by: Grant Likely grant.lik...@secretlab.ca
 Cc: devicetree-disc...@lists.ozlabs.org
 Cc: Ben Dooks ben-li...@fluff.org
 Cc: linux-i2c@vger.kernel.org
 ---

Any comments on this? If not, will you apply to your tree?

Rob

  Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 
 ++
  drivers/i2c/busses/i2c-designware.c  |8 +++
  2 files changed, 31 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt
 
 diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
 b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 new file mode 100644
 index 000..cbcb404
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 @@ -0,0 +1,23 @@
 +* Synopsys DesignWare I2C
 +
 +Required properties :
 +
 + - compatible : should be snps,designware-i2c
 + - reg : Offset and length of the register set for the device
 + - interrupts : IRQ where IRQ is the interrupt number.
 +
 +Recommended properties :
 +
 + - clock-frequency : desired I2C bus clock frequency in Hz.
 +
 +Example :
 +
 + i2c@f {
 + #address-cells = 1;
 + #size-cells = 0;
 + compatible = snps,designware-i2c;
 + reg = 0xf 0x1000;
 + interrupts = 11;
 + clock-frequency = 40;
 + };
 +
 diff --git a/drivers/i2c/busses/i2c-designware.c 
 b/drivers/i2c/busses/i2c-designware.c
 index b7a51c4..0223b98 100644
 --- a/drivers/i2c/busses/i2c-designware.c
 +++ b/drivers/i2c/busses/i2c-designware.c
 @@ -769,6 +769,7 @@ static int __devinit dw_i2c_probe(struct platform_device 
 *pdev)
   sizeof(adap-name));
   adap-algo = i2c_dw_algo;
   adap-dev.parent = pdev-dev;
 + adap-dev.of_node = pdev-dev.of_node;
  
   adap-nr = pdev-id;
   r = i2c_add_numbered_adapter(adap);
 @@ -819,6 +820,12 @@ static int __devexit dw_i2c_remove(struct 
 platform_device *pdev)
   return 0;
  }
  
 +static const struct of_device_id dw_i2c_of_match[] = {
 + { .compatible = snps,designware-i2c, },
 + {},
 +};
 +MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 +
  /* work with hotplug and coldplug */
  MODULE_ALIAS(platform:i2c_designware);
  
 @@ -827,6 +834,7 @@ static struct platform_driver dw_i2c_driver = {
   .driver = {
   .name   = i2c_designware,
   .owner  = THIS_MODULE,
 + .of_match_table = dw_i2c_of_match,
   },
  };
  

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt: add empty of_get_property for non-dt

[Rob Herring]

On 08/05/2011 05:50 PM, Stephen Warren wrote:
 The patch adds empty function of_get_property for non-dt build, so that
 drivers migrating to dt can save some '#ifdef CONFIG_OF'.
 
 This also fixes the current Tegra compile problem in linux-next.
 
You could just use of_property_read_u32 in the driver. It already has
empty version and will simplify the driver code some.

Rob

 Signed-off-by: Stephen Warren swar...@nvidia.com
 ---
  include/linux/of.h |7 +++
  1 files changed, 7 insertions(+), 0 deletions(-)
 
 diff --git a/include/linux/of.h b/include/linux/of.h
 index 0085bb0..9180dc5 100644
 --- a/include/linux/of.h
 +++ b/include/linux/of.h
 @@ -256,6 +256,13 @@ static inline int of_property_read_string(struct 
 device_node *np,
   return -ENOSYS;
  }
  
 +static inline const void *of_get_property(const struct device_node *node,
 + const char *name,
 + int *lenp)
 +{
 + return NULL;
 +}
 +
  #endif /* CONFIG_OF */
  
  static inline int of_property_read_u32(const struct device_node *np,

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] Move OF i2c functions into i2c core

[Rob Herring]

From: Rob Herring rob.herr...@calxeda.com

As suggested by Grant, this series moves OF i2c functions in of_i2c.c into
i2c-core.c. This simplifies OF i2c drivers slightly. Combining the
functions is necessary to avoid a module circular dependency between
i2c-core.ko and of_i2c.ko.

Compile tested on ARM (DT/non-DT VExpress) and PowerPC (8xxx and 44x).

Rob

Rob Herring (3):
  i2c: move of_i2c_register_devices call into core
  i2c: move OF i2c related functions into i2c core
  i2c-designware: add OF binding support

 Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 +
 arch/powerpc/platforms/44x/warp.c|1 -
 drivers/i2c/busses/i2c-cpm.c |6 --
 drivers/i2c/busses/i2c-designware.c  |8 ++
 drivers/i2c/busses/i2c-ibm_iic.c |4 -
 drivers/i2c/busses/i2c-mpc.c |2 -
 drivers/i2c/busses/i2c-pxa.c |2 -
 drivers/i2c/busses/i2c-tegra.c   |3 -
 drivers/i2c/i2c-core.c   |   87 +++-
 drivers/of/Makefile  |1 -
 drivers/of/of_i2c.c  |   97 --
 include/linux/i2c.h  |4 +
 include/linux/of_i2c.h   |   30 ---
 13 files changed, 121 insertions(+), 147 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt
 delete mode 100644 drivers/of/of_i2c.c
 delete mode 100644 include/linux/of_i2c.h

-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] i2c: move of_i2c_register_devices call into core

[Rob Herring]

From: Rob Herring rob.herr...@calxeda.com

All callers of of_i2c_register_devices are immediately preceded by a call
to i2c_add_adapter or i2c_add_numbered_adapter. Add call to
of_i2c_register_devices and remove all other callers.

This causes a module dependency loop that is resolved by the next commit.

Signed-off-by: Rob Herring rob.herr...@calxeda.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Jochen Friedrich joc...@scram.de
Cc: Jean Delvare kh...@linux-fr.org
Cc: Ben Dooks ben-li...@fluff.org
Cc: Sebastian Andrzej Siewior bige...@linutronix.de
Cc: Dirk Brandewie dirk.brande...@gmail.com
Cc: Stephen Warren swar...@nvidia.com
Cc: linuxppc-...@lists.ozlabs.org
Cc: linux-i2c@vger.kernel.org
---
 drivers/i2c/busses/i2c-cpm.c |6 --
 drivers/i2c/busses/i2c-ibm_iic.c |4 
 drivers/i2c/busses/i2c-mpc.c |2 --
 drivers/i2c/busses/i2c-pxa.c |2 --
 drivers/i2c/busses/i2c-tegra.c   |3 ---
 drivers/i2c/i2c-core.c   |4 
 6 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index b1d9cd2..7cbc82a 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -42,7 +42,6 @@
 #include linux/dma-mapping.h
 #include linux/of_device.h
 #include linux/of_platform.h
-#include linux/of_i2c.h
 #include sysdev/fsl_soc.h
 #include asm/cpm.h
 
@@ -673,11 +672,6 @@ static int __devinit cpm_i2c_probe(struct platform_device 
*ofdev)
dev_dbg(ofdev-dev, hw routines for %s registered.\n,
cpm-adap.name);
 
-   /*
-* register OF I2C devices
-*/
-   of_i2c_register_devices(cpm-adap);
-
return 0;
 out_shut:
cpm_i2c_shutdown(cpm);
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 3c110fb..5ba15ba 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -42,7 +42,6 @@
 #include linux/io.h
 #include linux/i2c.h
 #include linux/of_platform.h
-#include linux/of_i2c.h
 
 #include i2c-ibm_iic.h
 
@@ -759,9 +758,6 @@ static int __devinit iic_probe(struct platform_device 
*ofdev)
dev_info(ofdev-dev, using %s mode\n,
 dev-fast_mode ? fast (400 kHz) : standard (100 kHz));
 
-   /* Now register all the child nodes */
-   of_i2c_register_devices(adap);
-
return 0;
 
 error_cleanup:
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 107397a..a433f59 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -18,7 +18,6 @@
 #include linux/sched.h
 #include linux/init.h
 #include linux/of_platform.h
-#include linux/of_i2c.h
 #include linux/slab.h
 
 #include linux/io.h
@@ -637,7 +636,6 @@ static int __devinit fsl_i2c_probe(struct platform_device 
*op)
dev_err(i2c-dev, failed to add adapter\n);
goto fail_add;
}
-   of_i2c_register_devices(i2c-adap);
 
return result;
 
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index d603646..c1c2885 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -29,7 +29,6 @@
 #include linux/errno.h
 #include linux/interrupt.h
 #include linux/i2c-pxa.h
-#include linux/of_i2c.h
 #include linux/platform_device.h
 #include linux/err.h
 #include linux/clk.h
@@ -1147,7 +1146,6 @@ static int i2c_pxa_probe(struct platform_device *dev)
printk(KERN_INFO I2C: Failed to add bus\n);
goto eadapt;
}
-   of_i2c_register_devices(i2c-adap);
 
platform_set_drvdata(dev, i2c);
 
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 2440b74..9ec0a58 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -26,7 +26,6 @@
 #include linux/delay.h
 #include linux/slab.h
 #include linux/i2c-tegra.h
-#include linux/of_i2c.h
 
 #include asm/unaligned.h
 
@@ -653,8 +652,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
goto err_free_irq;
}
 
-   of_i2c_register_devices(i2c_dev-adapter);
-
return 0;
 err_free_irq:
free_irq(i2c_dev-irq, i2c_dev);
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 131079a..011e195 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -32,6 +32,7 @@
 #include linux/init.h
 #include linux/idr.h
 #include linux/mutex.h
+#include linux/of_i2c.h
 #include linux/of_device.h
 #include linux/completion.h
 #include linux/hardirq.h
@@ -863,6 +864,9 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
if (adap-nr  __i2c_first_dynamic_bus_num)
i2c_scan_static_board_info(adap);
 
+   /* Register devices from the device tree */
+   of_i2c_register_devices(adap);
+
/* Notify drivers */
mutex_lock(core_lock);
bus_for_each_drv(i2c_bus_type, NULL, adap, __process_new_adapter);
-- 
1.7.4.1

--
To unsubscribe from this list

[PATCH 3/3] i2c-designware: add OF binding support

[Rob Herring]

From: Rob Herring rob.herr...@calxeda.com

Add of_match_table and DT style i2c registration to designware i2c
driver.

Signed-off-by: Rob Herring rob.herr...@calxeda.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Ben Dooks ben-li...@fluff.org
Cc: linux-i2c@vger.kernel.org
---
 Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 ++
 drivers/i2c/busses/i2c-designware.c  |8 +++
 2 files changed, 31 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt

diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
new file mode 100644
index 000..cbcb404
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
@@ -0,0 +1,23 @@
+* Synopsys DesignWare I2C
+
+Required properties :
+
+ - compatible : should be snps,designware-i2c
+ - reg : Offset and length of the register set for the device
+ - interrupts : IRQ where IRQ is the interrupt number.
+
+Recommended properties :
+
+ - clock-frequency : desired I2C bus clock frequency in Hz.
+
+Example :
+
+   i2c@f {
+   #address-cells = 1;
+   #size-cells = 0;
+   compatible = snps,designware-i2c;
+   reg = 0xf 0x1000;
+   interrupts = 11;
+   clock-frequency = 40;
+   };
+
diff --git a/drivers/i2c/busses/i2c-designware.c 
b/drivers/i2c/busses/i2c-designware.c
index b7a51c4..0223b98 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -769,6 +769,7 @@ static int __devinit dw_i2c_probe(struct platform_device 
*pdev)
sizeof(adap-name));
adap-algo = i2c_dw_algo;
adap-dev.parent = pdev-dev;
+   adap-dev.of_node = pdev-dev.of_node;
 
adap-nr = pdev-id;
r = i2c_add_numbered_adapter(adap);
@@ -819,6 +820,12 @@ static int __devexit dw_i2c_remove(struct platform_device 
*pdev)
return 0;
 }
 
+static const struct of_device_id dw_i2c_of_match[] = {
+   { .compatible = snps,designware-i2c, },
+   {},
+};
+MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
+
 /* work with hotplug and coldplug */
 MODULE_ALIAS(platform:i2c_designware);
 
@@ -827,6 +834,7 @@ static struct platform_driver dw_i2c_driver = {
.driver = {
.name   = i2c_designware,
.owner  = THIS_MODULE,
+   .of_match_table = dw_i2c_of_match,
},
 };
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] i2c: move OF i2c related functions into i2c core

[Rob Herring]

From: Rob Herring rob.herr...@calxeda.com

This simplifies i2c drivers by removing calls to of_i2c_register_devices
and resolves a module circular dependency between i2c-core and of_i2c.

Signed-off-by: Rob Herring rob.herr...@calxeda.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Jean Delvare kh...@linux-fr.org
Cc: Ben Dooks ben-li...@fluff.org
Cc: linux-i2c@vger.kernel.org
---
 arch/powerpc/platforms/44x/warp.c |1 -
 drivers/i2c/i2c-core.c|   85 +++-
 drivers/of/Makefile   |1 -
 drivers/of/of_i2c.c   |   97 -
 include/linux/i2c.h   |4 ++
 include/linux/of_i2c.h|   30 ---
 6 files changed, 87 insertions(+), 131 deletions(-)
 delete mode 100644 drivers/of/of_i2c.c
 delete mode 100644 include/linux/of_i2c.h

diff --git a/arch/powerpc/platforms/44x/warp.c 
b/arch/powerpc/platforms/44x/warp.c
index 8f77139..9327ccf 100644
--- a/arch/powerpc/platforms/44x/warp.c
+++ b/arch/powerpc/platforms/44x/warp.c
@@ -16,7 +16,6 @@
 #include linux/interrupt.h
 #include linux/delay.h
 #include linux/of_gpio.h
-#include linux/of_i2c.h
 #include linux/slab.h
 
 #include asm/machdep.h
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 011e195..478c7f2 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -22,7 +22,10 @@
SMBus 2.0 support by Mark Studebaker mdsxyz...@yahoo.com and
Jean Delvare kh...@linux-fr.org
Mux support by Rodolfo Giometti giome...@enneenne.com and
-   Michael Lawnick michael.lawnick@nsn.com */
+   Michael Lawnick michael.lawnick@nsn.com
+   OF support by Jochen Friedrich joc...@scram.de and
+   Jon Smirl jonsm...@gmail.com
+   */
 
 #include linux/module.h
 #include linux/kernel.h
@@ -32,7 +35,6 @@
 #include linux/init.h
 #include linux/idr.h
 #include linux/mutex.h
-#include linux/of_i2c.h
 #include linux/of_device.h
 #include linux/completion.h
 #include linux/hardirq.h
@@ -790,6 +792,85 @@ static void i2c_scan_static_board_info(struct i2c_adapter 
*adapter)
up_read(__i2c_board_lock);
 }
 
+#ifdef CONFIG_OF_I2C
+static void of_i2c_register_devices(struct i2c_adapter *adap)
+{
+   void *result;
+   struct device_node *node;
+
+   /* Only register child devices if the adapter has a node pointer set */
+   if (!adap-dev.of_node)
+   return;
+
+   dev_dbg(adap-dev, of_i2c: walking child nodes\n);
+
+   for_each_child_of_node(adap-dev.of_node, node) {
+   struct i2c_board_info info = {};
+   struct dev_archdata dev_ad = {};
+   const __be32 *addr;
+   int len;
+
+   dev_dbg(adap-dev, of_i2c: register %s\n, node-full_name);
+
+   if (of_modalias_node(node, info.type, sizeof(info.type))  0) {
+   dev_err(adap-dev, of_i2c: modalias failure on %s\n,
+   node-full_name);
+   continue;
+   }
+
+   addr = of_get_property(node, reg, len);
+   if (!addr || (len  sizeof(int))) {
+   dev_err(adap-dev, of_i2c: invalid reg on %s\n,
+   node-full_name);
+   continue;
+   }
+
+   info.addr = be32_to_cpup(addr);
+   if (info.addr  (1  10) - 1) {
+   dev_err(adap-dev, of_i2c: invalid addr=%x on %s\n,
+   info.addr, node-full_name);
+   continue;
+   }
+
+   info.irq = irq_of_parse_and_map(node, 0);
+   info.of_node = of_node_get(node);
+   info.archdata = dev_ad;
+
+   request_module(%s%s, I2C_MODULE_PREFIX, info.type);
+
+   result = i2c_new_device(adap, info);
+   if (result == NULL) {
+   dev_err(adap-dev, of_i2c: Failure registering %s\n,
+   node-full_name);
+   of_node_put(node);
+   irq_dispose_mapping(info.irq);
+   continue;
+   }
+   }
+}
+
+static int of_dev_node_match(struct device *dev, void *data)
+{
+return dev-of_node == data;
+}
+
+/* must call put_device() when done with returned i2c_client device */
+struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
+{
+   struct device *dev;
+
+   dev = bus_find_device(i2c_bus_type, NULL, node,
+of_dev_node_match);
+   if (!dev)
+   return NULL;
+
+   return to_i2c_client(dev);
+}
+EXPORT_SYMBOL(of_find_i2c_device_by_node);
+#else
+static void of_i2c_register_devices(struct i2c_adapter *adap) {}
+#endif
+
 static int i2c_do_add_adapter(struct i2c_driver *driver,
  struct i2c_adapter *adap)
 {
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index dccb117

Re: [PATCH 2/3] i2c: move OF i2c related functions into i2c core

[Rob Herring]

Grant,

On 08/05/2011 05:56 PM, Grant Likely wrote:
 On Fri, Aug 05, 2011 at 04:24:27PM -0500, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com

 This simplifies i2c drivers by removing calls to of_i2c_register_devices
 and resolves a module circular dependency between i2c-core and of_i2c.

 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: Jean Delvare kh...@linux-fr.org
 Cc: Ben Dooks ben-li...@fluff.org
 Cc: linux-i2c@vger.kernel.org
 ---
  arch/powerpc/platforms/44x/warp.c |1 -
  drivers/i2c/i2c-core.c|   85 +++-
  drivers/of/Makefile   |1 -
  drivers/of/of_i2c.c   |   97 
 -
  include/linux/i2c.h   |4 ++
  include/linux/of_i2c.h|   30 ---
  6 files changed, 87 insertions(+), 131 deletions(-)
  delete mode 100644 drivers/of/of_i2c.c
  delete mode 100644 include/linux/of_i2c.h

 diff --git a/arch/powerpc/platforms/44x/warp.c 
 b/arch/powerpc/platforms/44x/warp.c
 index 8f77139..9327ccf 100644
 --- a/arch/powerpc/platforms/44x/warp.c
 +++ b/arch/powerpc/platforms/44x/warp.c
 @@ -16,7 +16,6 @@
  #include linux/interrupt.h
  #include linux/delay.h
  #include linux/of_gpio.h
 -#include linux/of_i2c.h
  #include linux/slab.h
  
  #include asm/machdep.h
 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 index 011e195..478c7f2 100644
 --- a/drivers/i2c/i2c-core.c
 +++ b/drivers/i2c/i2c-core.c
 @@ -22,7 +22,10 @@
 SMBus 2.0 support by Mark Studebaker mdsxyz...@yahoo.com and
 Jean Delvare kh...@linux-fr.org
 Mux support by Rodolfo Giometti giome...@enneenne.com and
 -   Michael Lawnick michael.lawnick@nsn.com */
 +   Michael Lawnick michael.lawnick@nsn.com
 +   OF support by Jochen Friedrich joc...@scram.de and
 +   Jon Smirl jonsm...@gmail.com
 +   */
  
  #include linux/module.h
  #include linux/kernel.h
 @@ -32,7 +35,6 @@
  #include linux/init.h
  #include linux/idr.h
  #include linux/mutex.h
 -#include linux/of_i2c.h
  #include linux/of_device.h
  #include linux/completion.h
  #include linux/hardirq.h
 @@ -790,6 +792,85 @@ static void i2c_scan_static_board_info(struct 
 i2c_adapter *adapter)
  up_read(__i2c_board_lock);
  }
  
 +#ifdef CONFIG_OF_I2C
 
 I think this becomes simply #ifdef CONFIG_OF since CONFIG_I2C is implied at 
 this point.

I was going to remove CONFIG_OF_I2C altogether, but it depends on !SPARC
so I thought it should be kept.

 
 diff --git a/include/linux/i2c.h b/include/linux/i2c.h
 index a6c652e..830397b 100644
 --- a/include/linux/i2c.h
 +++ b/include/linux/i2c.h
 @@ -468,6 +468,10 @@ static inline int i2c_adapter_id(struct i2c_adapter 
 *adap)
  {
  return adap-nr;
  }
 +
 +/* must call put_device() when done with returned i2c_client device */
 +extern struct i2c_client *of_find_i2c_device_by_node(struct device_node 
 *node);
 +
 
 This shouldn't be defined if !CONFIG_OF

There is no empty version today and externs don't need to be ifdef'ed in
that case.

Rob

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] i2c: move of_i2c_register_devices call into core

[Rob Herring]

Grant,

On 08/05/2011 05:54 PM, Grant Likely wrote:
 On Fri, Aug 05, 2011 at 04:24:26PM -0500, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com

 All callers of of_i2c_register_devices are immediately preceded by a call
 to i2c_add_adapter or i2c_add_numbered_adapter. Add call to
 of_i2c_register_devices and remove all other callers.

 This causes a module dependency loop that is resolved by the next commit.
 
 Wrong way around.  Don't break bisectability.  I'll leave it up to Ben
 and Jean on weather or not they want to move this code.  I intend to
 do the same thing for spi/gpio, but I maintain those subsystems so I
 get to choose.  i2c is not up to me.
 

Well, I know, but it's only broken for bisect in the case of both being
modules. So it's only run-time brokenness. That's better than the 3
months it was broken before. I couldn't come up with a better way. The
choices I thought of were:

-temporarily exporting and adding of_i2c_register_devices to i2c.h and
then removing it. I'm not a fan of adding that churn.
-just combining it all into 1 patch.
-reverse the order and leave i2c device registration broken for 1
commit. Thinking some more about it, perhaps that is a bit better than
broken module loading. Guess it depends if you are doing modules or
built-in.

Rob

 g.
 

 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: Jochen Friedrich joc...@scram.de
 Cc: Jean Delvare kh...@linux-fr.org
 Cc: Ben Dooks ben-li...@fluff.org
 Cc: Sebastian Andrzej Siewior bige...@linutronix.de
 Cc: Dirk Brandewie dirk.brande...@gmail.com
 Cc: Stephen Warren swar...@nvidia.com
 Cc: linuxppc-...@lists.ozlabs.org
 Cc: linux-i2c@vger.kernel.org
 ---
  drivers/i2c/busses/i2c-cpm.c |6 --
  drivers/i2c/busses/i2c-ibm_iic.c |4 
  drivers/i2c/busses/i2c-mpc.c |2 --
  drivers/i2c/busses/i2c-pxa.c |2 --
  drivers/i2c/busses/i2c-tegra.c   |3 ---
  drivers/i2c/i2c-core.c   |4 
  6 files changed, 4 insertions(+), 17 deletions(-)

 diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
 index b1d9cd2..7cbc82a 100644
 --- a/drivers/i2c/busses/i2c-cpm.c
 +++ b/drivers/i2c/busses/i2c-cpm.c
 @@ -42,7 +42,6 @@
  #include linux/dma-mapping.h
  #include linux/of_device.h
  #include linux/of_platform.h
 -#include linux/of_i2c.h
  #include sysdev/fsl_soc.h
  #include asm/cpm.h
  
 @@ -673,11 +672,6 @@ static int __devinit cpm_i2c_probe(struct 
 platform_device *ofdev)
  dev_dbg(ofdev-dev, hw routines for %s registered.\n,
  cpm-adap.name);
  
 -/*
 - * register OF I2C devices
 - */
 -of_i2c_register_devices(cpm-adap);
 -
  return 0;
  out_shut:
  cpm_i2c_shutdown(cpm);
 diff --git a/drivers/i2c/busses/i2c-ibm_iic.c 
 b/drivers/i2c/busses/i2c-ibm_iic.c
 index 3c110fb..5ba15ba 100644
 --- a/drivers/i2c/busses/i2c-ibm_iic.c
 +++ b/drivers/i2c/busses/i2c-ibm_iic.c
 @@ -42,7 +42,6 @@
  #include linux/io.h
  #include linux/i2c.h
  #include linux/of_platform.h
 -#include linux/of_i2c.h
  
  #include i2c-ibm_iic.h
  
 @@ -759,9 +758,6 @@ static int __devinit iic_probe(struct platform_device 
 *ofdev)
  dev_info(ofdev-dev, using %s mode\n,
   dev-fast_mode ? fast (400 kHz) : standard (100 kHz));
  
 -/* Now register all the child nodes */
 -of_i2c_register_devices(adap);
 -
  return 0;
  
  error_cleanup:
 diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
 index 107397a..a433f59 100644
 --- a/drivers/i2c/busses/i2c-mpc.c
 +++ b/drivers/i2c/busses/i2c-mpc.c
 @@ -18,7 +18,6 @@
  #include linux/sched.h
  #include linux/init.h
  #include linux/of_platform.h
 -#include linux/of_i2c.h
  #include linux/slab.h
  
  #include linux/io.h
 @@ -637,7 +636,6 @@ static int __devinit fsl_i2c_probe(struct 
 platform_device *op)
  dev_err(i2c-dev, failed to add adapter\n);
  goto fail_add;
  }
 -of_i2c_register_devices(i2c-adap);
  
  return result;
  
 diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
 index d603646..c1c2885 100644
 --- a/drivers/i2c/busses/i2c-pxa.c
 +++ b/drivers/i2c/busses/i2c-pxa.c
 @@ -29,7 +29,6 @@
  #include linux/errno.h
  #include linux/interrupt.h
  #include linux/i2c-pxa.h
 -#include linux/of_i2c.h
  #include linux/platform_device.h
  #include linux/err.h
  #include linux/clk.h
 @@ -1147,7 +1146,6 @@ static int i2c_pxa_probe(struct platform_device *dev)
  printk(KERN_INFO I2C: Failed to add bus\n);
  goto eadapt;
  }
 -of_i2c_register_devices(i2c-adap);
  
  platform_set_drvdata(dev, i2c);
  
 diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
 index 2440b74..9ec0a58 100644
 --- a/drivers/i2c/busses/i2c-tegra.c
 +++ b/drivers/i2c/busses/i2c-tegra.c
 @@ -26,7 +26,6 @@
  #include linux/delay.h
  #include linux/slab.h
  #include linux/i2c-tegra.h
 -#include linux/of_i2c.h
  
  #include asm/unaligned.h

Re: [PATCH] i2c-designware: add OF binding support

[Rob Herring]

Ben,

On 08/04/2011 04:12 AM, Ben Dooks wrote:
 On Wed, Aug 03, 2011 at 03:04:23PM -0500, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com

 Add of_match_table and DT style i2c registration to designware i2c
 driver.

 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: devicetree-disc...@lists.ozlabs.org
 Cc: Ben Dooks ben-li...@fluff.org
 Cc: linux-i2c@vger.kernel.org
 ---
  Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 
 ++
  drivers/i2c/busses/i2c-designware.c  |   13 
  2 files changed, 36 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt

 diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
 b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 new file mode 100644
 index 000..cbcb404
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 @@ -0,0 +1,23 @@
 +* Synopsys DesignWare I2C
 +
 +Required properties :
 +
 + - compatible : should be snps,designware-i2c
 + - reg : Offset and length of the register set for the device
 + - interrupts : IRQ where IRQ is the interrupt number.
 +
 +Recommended properties :
 +
 + - clock-frequency : desired I2C bus clock frequency in Hz.
 +
 +Example :
 +
 +i2c@f {
 +#address-cells = 1;
 +#size-cells = 0;
 +compatible = snps,designware-i2c;
 +reg = 0xf 0x1000;
 +interrupts = 11;
 +clock-frequency = 40;
 +};
 +
 
 looks good to me.
 
 diff --git a/drivers/i2c/busses/i2c-designware.c 
 b/drivers/i2c/busses/i2c-designware.c
 index b7a51c4..2911a49 100644
 --- a/drivers/i2c/busses/i2c-designware.c
 +++ b/drivers/i2c/busses/i2c-designware.c
 @@ -37,6 +37,7 @@
  #include linux/platform_device.h
  #include linux/io.h
  #include linux/slab.h
 +#include linux/of_i2c.h
  
  /*
   * Registers offset
 @@ -770,12 +771,17 @@ static int __devinit dw_i2c_probe(struct 
 platform_device *pdev)
  adap-algo = i2c_dw_algo;
  adap-dev.parent = pdev-dev;
  
 +#ifdef CONFIG_OF
 +r = i2c_add_adapter(adap);
 +#else
  adap-nr = pdev-id;
  r = i2c_add_numbered_adapter(adap);
 +#endif
 
 I would say that doing the #ifdef CONFIG_OF is dangerous here when we
 are in a mixed OF/platform enviromnent as we're depending on compile
 time selection.
 
 I'm also wondering whether we have an of helper macro which takes
 a pdev and gives you an adapter number either given on pdev-id or
 -1 for the case when we're using the OF bindings.
 
 It might be worth talking to Grant about setting pdev-id to -1 if we
 are using an OF device.
 

As Grant said, that's already done and this hunk is not needed.

  if (r) {
  dev_err(pdev-dev, failure adding adapter\n);
  goto err_free_irq;
  }
 +of_i2c_register_devices(adap);
 
 If we did that, we could add a of_i2c_register_adapter() call which
 would take the platform device and then do the of_i2c_register_devices()
 and do these steps.
 

Better yet, how about putting of_i2c_register_devices into
i2c_register_adapter? Everywhere that calls of_i2c_register_devices is
preceded by a call to i2c_add_numbered_adapter or i2c_add_adapter. It
seems logical to put it with i2c_scan_static_board_info. I'll prepare a
patch to add that and remove all the other callers unless you think
that's a bad idea.

  return 0;
  
 @@ -819,6 +825,12 @@ static int __devexit dw_i2c_remove(struct 
 platform_device *pdev)
  return 0;
  }
  
 +static const struct of_device_id dw_i2c_of_match[] = {
 +{ .compatible = snps,designware-i2c, },
 +{},
 +};
 +MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 +
  /* work with hotplug and coldplug */
  MODULE_ALIAS(platform:i2c_designware);
  
 @@ -827,6 +839,7 @@ static struct platform_driver dw_i2c_driver = {
  .driver = {
  .name   = i2c_designware,
  .owner  = THIS_MODULE,
 +.of_match_table = dw_i2c_of_match,
 
 If my patch for of_match_ptr() is accepted, it will be needed here
 otherwise you will need to do something about remvoing the of table
 above if not config of.

There's really not much harm with having the table. If you match the
device in the non-DT way, the table is not used. Drivers should never
directly access the table either, but use the helpers to get their data.

Rob


--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c-designware: add OF binding support

[Rob Herring]

On 08/04/2011 10:45 AM, Rob Herring wrote:
 Ben,
 
 On 08/04/2011 04:12 AM, Ben Dooks wrote:
 On Wed, Aug 03, 2011 at 03:04:23PM -0500, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com

 Add of_match_table and DT style i2c registration to designware i2c
 driver.

 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: devicetree-disc...@lists.ozlabs.org
 Cc: Ben Dooks ben-li...@fluff.org
 Cc: linux-i2c@vger.kernel.org
 ---
  Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 
 ++
  drivers/i2c/busses/i2c-designware.c  |   13 
  2 files changed, 36 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt

 diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
 b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 new file mode 100644
 index 000..cbcb404
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 @@ -0,0 +1,23 @@
 +* Synopsys DesignWare I2C
 +
 +Required properties :
 +
 + - compatible : should be snps,designware-i2c
 + - reg : Offset and length of the register set for the device
 + - interrupts : IRQ where IRQ is the interrupt number.
 +
 +Recommended properties :
 +
 + - clock-frequency : desired I2C bus clock frequency in Hz.
 +
 +Example :
 +
 +   i2c@f {
 +   #address-cells = 1;
 +   #size-cells = 0;
 +   compatible = snps,designware-i2c;
 +   reg = 0xf 0x1000;
 +   interrupts = 11;
 +   clock-frequency = 40;
 +   };
 +

 looks good to me.

 diff --git a/drivers/i2c/busses/i2c-designware.c 
 b/drivers/i2c/busses/i2c-designware.c
 index b7a51c4..2911a49 100644
 --- a/drivers/i2c/busses/i2c-designware.c
 +++ b/drivers/i2c/busses/i2c-designware.c
 @@ -37,6 +37,7 @@
  #include linux/platform_device.h
  #include linux/io.h
  #include linux/slab.h
 +#include linux/of_i2c.h
  
  /*
   * Registers offset
 @@ -770,12 +771,17 @@ static int __devinit dw_i2c_probe(struct 
 platform_device *pdev)
 adap-algo = i2c_dw_algo;
 adap-dev.parent = pdev-dev;
  
 +#ifdef CONFIG_OF
 +   r = i2c_add_adapter(adap);
 +#else
 adap-nr = pdev-id;
 r = i2c_add_numbered_adapter(adap);
 +#endif

 I would say that doing the #ifdef CONFIG_OF is dangerous here when we
 are in a mixed OF/platform enviromnent as we're depending on compile
 time selection.

 I'm also wondering whether we have an of helper macro which takes
 a pdev and gives you an adapter number either given on pdev-id or
 -1 for the case when we're using the OF bindings.

 It might be worth talking to Grant about setting pdev-id to -1 if we
 are using an OF device.

 
 As Grant said, that's already done and this hunk is not needed.
 
 if (r) {
 dev_err(pdev-dev, failure adding adapter\n);
 goto err_free_irq;
 }
 +   of_i2c_register_devices(adap);

 If we did that, we could add a of_i2c_register_adapter() call which
 would take the platform device and then do the of_i2c_register_devices()
 and do these steps.

 
 Better yet, how about putting of_i2c_register_devices into
 i2c_register_adapter? Everywhere that calls of_i2c_register_devices is
 preceded by a call to i2c_add_numbered_adapter or i2c_add_adapter. It
 seems logical to put it with i2c_scan_static_board_info. I'll prepare a
 patch to add that and remove all the other callers unless you think
 that's a bad idea.
 

Nevermind. That would be undoing this commit:

of/i2c: Fix module load order issue caused by of_i2c.c

Commit 959e85f7, i2c: add OF-style registration and binding caused a
module dependency loop where of_i2c.c calls functions in i2c-core, and
i2c-core calls of_i2c_register_devices() in of_i2c.  This means that
when i2c support is built as a module when CONFIG_OF is set, then
neither i2c_core nor of_i2c are able to be loaded.

This patch fixes the problem by moving the of_i2c_register_devices()
calls back into the device drivers.  Device drivers already
specifically request the core code to parse the device tree for
devices anyway by setting the of_node pointer, so it isn't a big
deal to also call the registration function.  The drivers just become
slightly more verbose.

Signed-off-by: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Jean Delvare kh...@linux-fr.org

Rob

 return 0;
  
 @@ -819,6 +825,12 @@ static int __devexit dw_i2c_remove(struct 
 platform_device *pdev)
 return 0;
  }
  
 +static const struct of_device_id dw_i2c_of_match[] = {
 +   { .compatible = snps,designware-i2c, },
 +   {},
 +};
 +MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 +
  /* work with hotplug and coldplug */
  MODULE_ALIAS(platform:i2c_designware);
  
 @@ -827,6 +839,7 @@ static struct platform_driver dw_i2c_driver = {
 .driver = {
 .name   = i2c_designware,
 .owner  = THIS_MODULE,
 +   .of_match_table = dw_i2c_of_match

[PATCH] i2c-designware: add OF binding support

[Rob Herring]

From: Rob Herring rob.herr...@calxeda.com

Add of_match_table and DT style i2c registration to designware i2c
driver.

Signed-off-by: Rob Herring rob.herr...@calxeda.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Ben Dooks ben-li...@fluff.org
Cc: linux-i2c@vger.kernel.org
---
 Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 ++
 drivers/i2c/busses/i2c-designware.c  |   13 
 2 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt

diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
new file mode 100644
index 000..cbcb404
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
@@ -0,0 +1,23 @@
+* Synopsys DesignWare I2C
+
+Required properties :
+
+ - compatible : should be snps,designware-i2c
+ - reg : Offset and length of the register set for the device
+ - interrupts : IRQ where IRQ is the interrupt number.
+
+Recommended properties :
+
+ - clock-frequency : desired I2C bus clock frequency in Hz.
+
+Example :
+
+   i2c@f {
+   #address-cells = 1;
+   #size-cells = 0;
+   compatible = snps,designware-i2c;
+   reg = 0xf 0x1000;
+   interrupts = 11;
+   clock-frequency = 40;
+   };
+
diff --git a/drivers/i2c/busses/i2c-designware.c 
b/drivers/i2c/busses/i2c-designware.c
index b7a51c4..2911a49 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -37,6 +37,7 @@
 #include linux/platform_device.h
 #include linux/io.h
 #include linux/slab.h
+#include linux/of_i2c.h
 
 /*
  * Registers offset
@@ -770,12 +771,17 @@ static int __devinit dw_i2c_probe(struct platform_device 
*pdev)
adap-algo = i2c_dw_algo;
adap-dev.parent = pdev-dev;
 
+#ifdef CONFIG_OF
+   r = i2c_add_adapter(adap);
+#else
adap-nr = pdev-id;
r = i2c_add_numbered_adapter(adap);
+#endif
if (r) {
dev_err(pdev-dev, failure adding adapter\n);
goto err_free_irq;
}
+   of_i2c_register_devices(adap);
 
return 0;
 
@@ -819,6 +825,12 @@ static int __devexit dw_i2c_remove(struct platform_device 
*pdev)
return 0;
 }
 
+static const struct of_device_id dw_i2c_of_match[] = {
+   { .compatible = snps,designware-i2c, },
+   {},
+};
+MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
+
 /* work with hotplug and coldplug */
 MODULE_ALIAS(platform:i2c_designware);
 
@@ -827,6 +839,7 @@ static struct platform_driver dw_i2c_driver = {
.driver = {
.name   = i2c_designware,
.owner  = THIS_MODULE,
+   .of_match_table = dw_i2c_of_match,
},
 };
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html