Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-17 Thread Mark Brown
On Fri, Feb 15, 2013 at 08:56:42PM +0100, Linus Walleij wrote: > Hm, totally unrelated but Mark, should we consider marking the > kernel tainted when the dummy regulators are in use? > (And likewise for the dummy pinctrl I guess.) It's not a bad idea, though since it tends not to result in kernel

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-15 Thread Linus Walleij
On Fri, Feb 15, 2013 at 1:19 AM, Stephen Warren wrote: > On 02/14/2013 05:02 PM, Linus Walleij wrote: >> On Thu, Feb 14, 2013 at 6:05 PM, Doug Anderson wrote: > ... >>> One argument for keeping "cd-inverted" too is >>> for controllers that don't use a GPIO for card detect. >>> In this case >>>

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-15 Thread Doug Anderson
Mark / Stephen, On Fri, Feb 15, 2013 at 2:24 AM, Mark Brown wrote: > You shouldn't have dummy regulators enabled at all, that'll break a > bunch of stuff including dependency resolution. They're a crutch to > help get things booting not something you should be using in production. > If they're n

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-15 Thread Linus Walleij
On Fri, Feb 15, 2013 at 9:42 PM, Stephen Warren wrote: > On 02/15/2013 01:34 PM, Linus Walleij wrote: >> 0xca00 + 4 is just the second 32bit word in the system >> controller. This address range and that very word is used >> for various stuff, so it's not like a general-purpose GPIO >> or anyt

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-15 Thread Stephen Warren
On 02/15/2013 01:34 PM, Linus Walleij wrote: > On Fri, Feb 15, 2013 at 1:19 AM, Stephen Warren wrote: >> On 02/14/2013 05:02 PM, Linus Walleij wrote: >>> On Thu, Feb 14, 2013 at 6:05 PM, Doug Anderson >>> wrote: >> ... One argument for keeping "cd-inverted" too is for controllers that

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-15 Thread Linus Walleij
On Fri, Feb 15, 2013 at 11:24 AM, Mark Brown wrote: > On Thu, Feb 14, 2013 at 01:40:49PM -0800, Doug Anderson wrote: >> On Wed, Feb 13, 2013 at 4:54 PM, Doug Anderson wrote: > >> ...but when I moved to module_platform_driver() then things still broke. > >> [1.51] platform-lcd supply lcd_v

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-15 Thread Mark Brown
On Thu, Feb 14, 2013 at 05:14:35PM -0800, Doug Anderson wrote: > The regulator_get() calls are before regulator_init_complete() by > quite a margin. The regulator_init_complete() is a late_initcall, so > that's not too surprising. Are we not supposed to access regulators > until after late_init(

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-15 Thread Mark Brown
On Thu, Feb 14, 2013 at 01:40:49PM -0800, Doug Anderson wrote: > On Wed, Feb 13, 2013 at 4:54 PM, Doug Anderson wrote: > ...but when I moved to module_platform_driver() then things still broke. > [1.51] platform-lcd supply lcd_vdd not found, using dummy regulator > I was sorta hoping th

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-14 Thread Alex Courbot
On Thu, Feb 14, 2013 at 2:01 AM, Linus Walleij wrote: On Thu, Feb 14, 2013 at 1:41 AM, Stephen Warren wrote: On 02/13/2013 05:34 PM, Doug Anderson wrote: A little torn here. It adds a bunch of complexity to the code to handle this case and there are no known or anticipated users. I only w

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-14 Thread Doug Anderson
Stephen, On Thu, Feb 14, 2013 at 4:16 PM, Stephen Warren wrote: >> We can avoid that logic with has_full_constraints. That will be some >> work to get done but also should be done at some point in time. Once >> we use has_full_constraints we'll get ERR_PTR(-EPROBE_DEFER); > > That flag is norma

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-14 Thread Linus Walleij
On Thu, Feb 14, 2013 at 6:48 PM, Stephen Warren wrote: > This is actually why I rather dislike the idea that device tree has an > immutable bindings. To some exten, I imagine that much of > Documentation/stable_api_nonsense.txt could apply against device tree! > We can't fix problems like this by

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-14 Thread Stephen Warren
On 02/14/2013 05:02 PM, Linus Walleij wrote: > On Thu, Feb 14, 2013 at 6:05 PM, Doug Anderson wrote: ... >> One argument for keeping "cd-inverted" too is >> for controllers that don't use a GPIO for card detect. >> In this case >> you could imagine a MMC controller that has a "card detect" on >>

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-14 Thread Stephen Warren
On 02/14/2013 04:59 PM, Doug Anderson wrote: > Stephen, > > On Thu, Feb 14, 2013 at 3:35 PM, Stephen Warren wrote: >>> [1.51] platform-lcd supply lcd_vdd not found, using dummy regulator >> >> What prints that? I assume that's some error-handling logic in the >> platform-lcd driver. It's

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-14 Thread Linus Walleij
On Thu, Feb 14, 2013 at 6:05 PM, Doug Anderson wrote: > Linus, >> But changing it would have very drastic effects. >> Consider this snippet from >> arch/arm/boot/dts/snowball.dts: >> >> // External Micro SD slot >> sdi0_per1@80126000 { >> arm,primecell-periphid = <0x10480180>; >> max-

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-14 Thread Doug Anderson
Stephen, On Thu, Feb 14, 2013 at 3:35 PM, Stephen Warren wrote: >> [1.51] platform-lcd supply lcd_vdd not found, using dummy regulator > > What prints that? I assume that's some error-handling logic in the > platform-lcd driver. It's probably not detecting an -EPROBE_DEFFERED > return fro

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-14 Thread Stephen Warren
On 02/14/2013 02:40 PM, Doug Anderson wrote: > On Wed, Feb 13, 2013 at 4:54 PM, Doug Anderson wrote: > You should be able to replace all that with: > > module_platform_driver(&i2c_arbitrator_driver); ... > OK, so I dug into my problems here a little bit. All of the problems > are with

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-14 Thread Doug Anderson
On Wed, Feb 13, 2013 at 4:54 PM, Doug Anderson wrote: You should be able to replace all that with: module_platform_driver(&i2c_arbitrator_driver); >>> >>> Sigh. Yeah, I had that. ...but it ends up getting initted >>> significantly later in the init process and that was uncovering

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-14 Thread Mark Brown
On Thu, Feb 14, 2013 at 10:48:45AM -0700, Stephen Warren wrote: > This is actually why I rather dislike the idea that device tree has an > immutable bindings. To some exten, I imagine that much of > Documentation/stable_api_nonsense.txt could apply against device tree! > We can't fix problems like

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-14 Thread Stephen Warren
On 02/14/2013 10:05 AM, Doug Anderson wrote: > Linus, > > On Thu, Feb 14, 2013 at 2:01 AM, Linus Walleij > wrote: >> On Thu, Feb 14, 2013 at 1:41 AM, Stephen Warren >> wrote: >>> On 02/13/2013 05:34 PM, Doug Anderson wrote: >> A little torn here. It adds a bunch of complexity to the code

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-14 Thread Stephen Warren
On 02/14/2013 03:01 AM, Linus Walleij wrote: > On Thu, Feb 14, 2013 at 1:41 AM, Stephen Warren wrote: >> On 02/13/2013 05:34 PM, Doug Anderson wrote: > >>> A little torn here. It adds a bunch of complexity to the code to >>> handle this case and there are no known or anticipated users. I only >

Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver)

2013-02-14 Thread Doug Anderson
Linus, On Thu, Feb 14, 2013 at 2:01 AM, Linus Walleij wrote: > On Thu, Feb 14, 2013 at 1:41 AM, Stephen Warren wrote: >> On 02/13/2013 05:34 PM, Doug Anderson wrote: > >>> A little torn here. It adds a bunch of complexity to the code to >>> handle this case and there are no known or anticipated

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-14 Thread Linus Walleij
On Thu, Feb 14, 2013 at 1:41 AM, Stephen Warren wrote: > On 02/13/2013 05:34 PM, Doug Anderson wrote: >> A little torn here. It adds a bunch of complexity to the code to >> handle this case and there are no known or anticipated users. I only >> wish that the GPIO polarity could be more hidden f

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-13 Thread Doug Anderson
Stephen, On Wed, Feb 13, 2013 at 4:41 PM, Stephen Warren wrote: >> OK, going to go with i2c-arbitrator-cros-ec. Hopefully that sounds OK. > > That seems fine. The mechanism seems potentially a little more generic > than just for cros-ec though, but I guess there's no harm naming it > after the f

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-13 Thread Stephen Warren
On 02/13/2013 05:34 PM, Doug Anderson wrote: > On Wed, Feb 13, 2013 at 1:02 PM, Stephen Warren wrote: >> On 02/13/2013 11:02 AM, Doug Anderson wrote: >>> The i2c-arbitrator driver implements the arbitration scheme that the >>> Embedded Controller (EC) on the ARM Chromebook expects to use for bus >

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-13 Thread Doug Anderson
Stephen, Thank you for the review. Comments below (including changes I have done locally). I probably won't have time to test / repost until tomorrow. On Wed, Feb 13, 2013 at 1:02 PM, Stephen Warren wrote: > On 02/13/2013 11:02 AM, Doug Anderson wrote: >> The i2c-arbitrator driver implements

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-13 Thread Stephen Warren
On 02/13/2013 11:02 AM, Doug Anderson wrote: > The i2c-arbitrator driver implements the arbitration scheme that the > Embedded Controller (EC) on the ARM Chromebook expects to use for bus > multimastering. This i2c-arbitrator driver could also be used in > other places where standard i2c bus arbit

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-13 Thread Olof Johansson
On Wed, Feb 13, 2013 at 10:45 AM, Olof Johansson wrote: >> + i2c-parent = <&{/i2c@12CA}>; > > Why use this custom i2c-parent property? The arbitrator should just be > under the i2c controller in the device tree, and thus parent would be > derived from the topology there. Ah. Read

Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-13 Thread Olof Johansson
Hi, The driver pieces have been reviewed a bit already, but I have a question on the bindings below. On Wed, Feb 13, 2013 at 10:02:09AM -0800, Doug Anderson wrote: > The i2c-arbitrator driver implements the arbitration scheme that the > Embedded Controller (EC) on the ARM Chromebook expects to us

[PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

2013-02-13 Thread Doug Anderson
The i2c-arbitrator driver implements the arbitration scheme that the Embedded Controller (EC) on the ARM Chromebook expects to use for bus multimastering. This i2c-arbitrator driver could also be used in other places where standard i2c bus arbitration can't be used and two extra GPIOs are availabl