| Message ID | 1363192583-26363-1-git-send-email-dianders@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Wolfram Sang |
| Headers | show |
On 03/13/2013 10:36 AM, Doug Anderson wrote: > The i2c-arbitrator-cros-ec driver implements the arbitration scheme > that the Embedded Controller (EC) on the ARM Chromebook expects to use > for bus multimastering. This i2c-arbitrator-cros-ec driver could also > be used in other places where standard I2C bus arbitration can't be > used and two extra GPIOs are available for arbitration. > > This driver is based on code that Simon Glass added to the i2c-s3c2410 > driver in the Chrome OS kernel 3.4 tree. The current incarnation as a > mux driver is as suggested by Grant Likely. See > <https://patchwork.kernel.org/patch/1877311/> for some history. ... > Changes in v4: None Isn't this 'PATCH V3 REPOST' then? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Stephen, On Wed, Mar 13, 2013 at 9:53 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> Changes in v4: None > > Isn't this 'PATCH V3 REPOST' then? In this case part 2 in the patch series changes but not parts 1 and 3. I could have just reposted part 2 with a higher version, but that makes it a little harder to piece together all of the parts of the series so I decided to repost all 3. I can do differently in the future if you prefer, but my understanding was that it was a matter of preference/judgement call. Thanks! :) -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/13/2013 10:59 AM, Doug Anderson wrote: > Stephen, > > On Wed, Mar 13, 2013 at 9:53 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> Changes in v4: None >> >> Isn't this 'PATCH V3 REPOST' then? > > In this case part 2 in the patch series changes but not parts 1 and 3. > I could have just reposted part 2 with a higher version, but that > makes it a little harder to piece together all of the parts of the > series so I decided to repost all 3. I can do differently in the > future if you prefer, but my understanding was that it was a matter of > preference/judgement call. Oh no you're quite right. I didn't notice it was a 3-part series, since I only got patch 1/3 filtered into my inbox; I guess I wasn't CC'd on the rest. Sorry for the noise. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/05/2013 01:37 PM, Simon Glass wrote: > HI Wolfram, > > On Wed, Apr 3, 2013 at 12:19 PM, Wolfram Sang <wsa@the-dreams.de> wrote: >> Doug, >> >>> Separately from a discussion of the technical merits, I'd say that >>> this patch is needed because the Embedded Controller (EC) on the ARM >>> Chromebook shipped expecting to communicate with this scheme. While >> >> Uhrm, with all respect, "we already shipped it" is not a convincing >> argument regarding inclusion. Benefit for the kernel is. I'm not quite sure why that isn't a convincing argument. The hardware has shipped. I don't know whether the EC microcode can be updated in the field; it seems risky to do so even if it's possible. So, it either gets supported or not; the HW/ucode isn't going to change I suspect. Hence, it seems that the decision would be: a) Disallow the implementation of the arbitration scheme in the kernel, and hence don't support this board in the kernel. (or at least some very core features of this board) b) Allow the implementation of the arbitration scheme in the kernel, and hence make possible support this board in the kernel. From that perspective, the benefit for the kernel question comes down to: do we see a benefit for the kernel to support this board? I can't see why that wouldn't be a benefit. The only disadvantage would be having to carrying code to support that board. That same argument can be made for any board, and I think typically doesn't cause any issue. The code for this I2C mux seems pretty self-contained, so even if it was absolutely terrible (which I don't think it is), it still wouldn't cause any wide-spread issues, I think. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 05, 2013 at 02:03:52PM -0600, Stephen Warren wrote: > On 04/05/2013 01:37 PM, Simon Glass wrote: > > HI Wolfram, > > > > On Wed, Apr 3, 2013 at 12:19 PM, Wolfram Sang <wsa@the-dreams.de> wrote: > >> Doug, > >> > >>> Separately from a discussion of the technical merits, I'd say that > >>> this patch is needed because the Embedded Controller (EC) on the ARM > >>> Chromebook shipped expecting to communicate with this scheme. While > >> > >> Uhrm, with all respect, "we already shipped it" is not a convincing > >> argument regarding inclusion. Benefit for the kernel is. > > I'm not quite sure why that isn't a convincing argument. > > The hardware has shipped. I don't know whether the EC microcode can be > updated in the field; it seems risky to do so even if it's possible. So, > it either gets supported or not; the HW/ucode isn't going to change I > suspect. > > Hence, it seems that the decision would be: > > a) Disallow the implementation of the arbitration scheme in the kernel, > and hence don't support this board in the kernel. (or at least some very > core features of this board) > > b) Allow the implementation of the arbitration scheme in the kernel, and > hence make possible support this board in the kernel. > > From that perspective, the benefit for the kernel question comes down > to: do we see a benefit for the kernel to support this board? I can't > see why that wouldn't be a benefit. > > The only disadvantage would be having to carrying code to support that > board. That same argument can be made for any board, and I think > typically doesn't cause any issue. The code for this I2C mux seems > pretty self-contained, so even if it was absolutely terrible (which I > don't think it is), it still wouldn't cause any wide-spread issues, I think. Very interesting discussion, especially the argument that "we already shipped" would not be a convincing argument. I had senior kernel maintainers tell me and the company I work for that we should submit _all_ our platform specific kernel code and drivers for inclusion into the upstream kernel. This exchange suggests that "it is a shipping product" does not count for such submissions, and that "Benefit for the kernel" would be the deciding factor instead. Which of course is a very vague statement - if code supporting Chromebookis is of no benefit for the kernel, support for my company's products for sure is much less so. Which kind of leaves me in a bind. On one side I promote that we should submit all our kernel code, on the other side there is a very compelling case to be made that it won't be accepted anyway. That doesn't make my life easier - essentially since it supports those who say that we should not submit anything in the first place. And believe me, there are many of those. Just to give some examples: - I2C multiplexers. We have a bunch of those. Looking at this exchange, it doesn't look good to get that code included. - Custom multi-function FPGAs and CPLDs, amongst others implementing I2C controllers, I2C muxes, GPIO access, Flash access, and other functions. Same as above. - Devicetree support for UIO devices (mostly forwarding ASICs), including gpio bindings, interrupt bindings, and clock bindings. Looking at older exchanges, that doesn't look good either. And please dont expect me to implement hacks around a clean solution because any devicetree binding for UIO drivers "does not describe hardware but its use". Now, I can understand that there may be technical or architectural issues preventing one driver or another from being accepted. For example, I can understand if a driver for an USB-I2C adapder isn't accepted because the adapter reports itself to the USB subsystem as serial driver. But "Benefit for the kernel" is vague enough to reject anything for no real reason other than someone not liking it (or the submitter, or the company the submitter works for, or the hardware architecture). It would be nice have to get some well defined guidelines for "acceptable" contributions. "Benefit for the kernel" sure isn't one. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > Very interesting discussion, especially the argument that "we already shipped" > would not be a convincing argument. > > I had senior kernel maintainers tell me and the company I work for that we should > submit _all_ our platform specific kernel code and drivers for inclusion into > the upstream kernel. Yes, great. Really! > This exchange suggests that "it is a shipping product" does not count for such > submissions, and that "Benefit for the kernel" would be the deciding factor > instead. Which of course is a very vague statement - if code supporting > Chromebookis is of no benefit for the kernel, support for my company's products > for sure is much less so. First, let me state that I did not intend to say that the arbitrator muxer here has NO benefit for the kernel. I was aware there might be arguments for that and I wanted some more discussion to make that clearer to me. Simon's mail was very helpful in that regard and I will comment on that somewhen later. Still, I do have problems with "we already shipped it". If the driver is bad or the underlying concept is fragile, I want the freedom to reject a patch, product shipped or not. I will be supportive to find a proper solution, promised. If all fails, there is still staging/ or the possibility of out-of-tree patches. > Which kind of leaves me in a bind. On one side I promote that we should submit > all our kernel code, on the other side there is a very compelling case to be > made that it won't be accepted anyway. That doesn't make my life easier - Concentrate on argumenting why the driver is useful and it will be fine. "we already shipped this" feels a bit like blackmailing to me. And since most drivers do have well thought reasons for their existance, I'd primarily like to hear about those. > essentially since it supports those who say that we should not submit anything > in the first place. And believe me, there are many of those. > > Just to give some examples: > - I2C multiplexers. We have a bunch of those. Looking at this exchange, > it doesn't look good to get that code included. Multiplexers should be easy going, it is the arbitration which is discussed here. I am open to consider some generic arbitration schemes. What I am reluctant to do is to allow every board to have its own arbitration scheme. This would be board support hosted in the I2C directory. Meh. > It would be nice have to get some well defined guidelines for "acceptable" > contributions. "Benefit for the kernel" sure isn't one. I don't think it is possible to write down concrete guidelines for this. "According to rule 4a) of the guidelines you have to accept my patch"? That would be a mess, I think. Regards, Wolfram -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Apr 06, 2013 at 10:11:32PM +0200, Wolfram Sang wrote: > Hi, > > > Very interesting discussion, especially the argument that "we already shipped" > > would not be a convincing argument. > > > > I had senior kernel maintainers tell me and the company I work for that we should > > submit _all_ our platform specific kernel code and drivers for inclusion into > > the upstream kernel. > > Yes, great. Really! > Yes, though, thinking about it, it was "submit" and didn't say anything about potential for acceptance. > > This exchange suggests that "it is a shipping product" does not count for such > > submissions, and that "Benefit for the kernel" would be the deciding factor > > instead. Which of course is a very vague statement - if code supporting > > Chromebookis is of no benefit for the kernel, support for my company's products > > for sure is much less so. > > First, let me state that I did not intend to say that the arbitrator > muxer here has NO benefit for the kernel. I was aware there might be > arguments for that and I wanted some more discussion to make that > clearer to me. Simon's mail was very helpful in that regard and I will > comment on that somewhen later. > > Still, I do have problems with "we already shipped it". If the driver is > bad or the underlying concept is fragile, I want the freedom to reject a > patch, product shipped or not. I will be supportive to find a proper > solution, promised. If all fails, there is still staging/ or the > possibility of out-of-tree patches. > I think there is a difference between a bad driver or underlying hardware. To me, "shipped" applies to hardware or firmware which can not be upgraded, not to the software running on it. > > Which kind of leaves me in a bind. On one side I promote that we should submit > > all our kernel code, on the other side there is a very compelling case to be > > made that it won't be accepted anyway. That doesn't make my life easier - > > Concentrate on argumenting why the driver is useful and it will be fine. > "we already shipped this" feels a bit like blackmailing to me. And since > most drivers do have well thought reasons for their existance, I'd > primarily like to hear about those. > > > essentially since it supports those who say that we should not submit anything > > in the first place. And believe me, there are many of those. > > > > Just to give some examples: > > - I2C multiplexers. We have a bunch of those. Looking at this exchange, > > it doesn't look good to get that code included. > > Multiplexers should be easy going, it is the arbitration which is discussed here. > I am open to consider some generic arbitration schemes. What I am reluctant to > do is to allow every board to have its own arbitration scheme. This > would be board support hosted in the I2C directory. Meh. > "board support hosted in the I2C directory". But that is exactly what I am talking about, isn't it ? I have board specific multiplexers and a board specific I2C controller, and that is just talking about the I2C code. > > It would be nice have to get some well defined guidelines for "acceptable" > > contributions. "Benefit for the kernel" sure isn't one. > > I don't think it is possible to write down concrete guidelines for this. > "According to rule 4a) of the guidelines you have to accept my patch"? > That would be a mess, I think. > Looking at it from a maintainer perspective, I agree. Where it gets murky is really the hardware part. The (in my opinion) philosophical arguments around not permitting device-tree based instantiation of uio devices is one example. Another practical example I had to deal with in my previous company is VGA memory space. Some hw geniuses decided to re-use the VGA memory space in an embedded x86 device for an EEPROM. Guess what - the x86 kernel writes into that space no matter what. A patch to address that problem was rejected because "you should not re-use VGA memory space". As if I had a choice. A better example might be Kontron board support. They implement gpio, I2C mux, and a watchdog in a PLD. They too have an access arbitration scheme where one has to acquire a hardware mutex before accessing the pld, if I understand correctly because some microcontroller might need to access it as well. Leaving the actual code aside, would you reject that too if you don't like the arbitration scheme, or because you don't want to have board support in the i2c directory ? Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Guenter, > I think there is a difference between a bad driver or underlying hardware. To > me, "shipped" applies to hardware or firmware which can not be upgraded, not to > the software running on it. OK. Valuable distinction. > "board support hosted in the I2C directory". But that is exactly what I am > talking about, isn't it ? I have board specific multiplexers and a board > specific I2C controller, and that is just talking about the I2C code. Yes and no. I think I can accept that some hardware has GPIOs wired to handle I2C arbitration. I still have problems in having arbitrator drivers per board, each one with various ideas how to do it. That would be a maintenance horror and has nothing to do with I2C, strictly speaking. What I would love to see is a few generic arbitrator drivers, e.g. utilizing a timeout-based scheme (as proposed here). Or like the old SCSI method putting IDs on the wire and the lowest wins. Stuff like that. > A better example might be Kontron board support. They implement gpio, I2C mux, > and a watchdog in a PLD. They too have an access arbitration scheme where > one has to acquire a hardware mutex before accessing the pld, if I understand > correctly because some microcontroller might need to access it as well. > Leaving the actual code aside, would you reject that too if you don't like > the arbitration scheme, or because you don't want to have board support > in the i2c directory ? If I understood correctly, getting the mutex would be done in some platform code and the I2C driver will simply call the necessary function to obtain and release the mutex. Or maybe the MFD layer can help, dunno. Both are fine with me and I don't care about the actual PLD arbitration. Thanks, Wolfram -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 13, 2013 at 09:36:21AM -0700, Doug Anderson wrote: > The i2c-arbitrator-cros-ec driver implements the arbitration scheme > that the Embedded Controller (EC) on the ARM Chromebook expects to use > for bus multimastering. This i2c-arbitrator-cros-ec driver could also > be used in other places where standard I2C bus arbitration can't be > used and two extra GPIOs are available for arbitration. > > This driver is based on code that Simon Glass added to the i2c-s3c2410 > driver in the Chrome OS kernel 3.4 tree. The current incarnation as a > mux driver is as suggested by Grant Likely. See > <https://patchwork.kernel.org/patch/1877311/> for some history. > > Signed-off-by: Doug Anderson <dianders@chromium.org> > Signed-off-by: Simon Glass <sjg@chromium.org> > Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com> > Reviewed-by: Stephen Warren <swarren@nvidia.com> > Tested-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com> I'd like to have the bindings more generic. They should allow for n possible masters IMO. It doesn't need to be implemented right now, but it should be possible to add that later. > --- > Changes in v4: None > Changes in v3: > - Handle of_find_i2c_adapter_by_node() failure more properly by > changing init order. > - Don't warn on -EPROBE_DEFER from calls that could return it. > - Move to module_platform_driver(). As we pull in parts of the system > that rely on devices under this i2c bus we'll need to make sure they > can handle the fact that they'll be initted later now. > > Changes in v2: > - Renamed to i2c-arbitrator-cros-ec. > - Documented "microsecond" properties as optional; removed > "bus-arbitration" prefix since it was just extra wordy. > - Split GPIOs into two properties to make it cleaner. > - Capitalized I2C in freeform text. > - Get 'active low' from device tree. > > .../bindings/i2c/i2c-arbitrator-cros-ec.txt | 76 +++++++ I wonder about a more generic name. i2c-arb-gpio-challenge.* maybe? > drivers/i2c/muxes/Kconfig | 11 + > drivers/i2c/muxes/Makefile | 2 + > drivers/i2c/muxes/i2c-arbitrator-cros-ec.c | 222 +++++++++++++++++++++ > 4 files changed, 311 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt > create mode 100644 drivers/i2c/muxes/i2c-arbitrator-cros-ec.c > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt > new file mode 100644 > index 0000000..1f893e7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt > @@ -0,0 +1,76 @@ > +GPIO-based Arbitration used by the ARM Chromebook (exynos5250-snow) > +=================================================================== > +This uses GPIO lines between the AP (Application Processor) and an attached > +EC (Embedded Controller) which both want to talk on the same I2C bus as master. > + > +The AP and EC each have a 'bus claim' line, which is an output that the > +other can see. These are both active low, with pull-ups enabled. > + > +- AP_CLAIM: output from AP, signalling to the EC that the AP wants the bus > +- EC_CLAIM: output from EC, signalling to the AP that the EC wants the bus I'd like to drop the specific terms of AP and EC and just talk about multiple masters. > +This mechanism is used instead of standard I2C multimaster to avoid some of the > +subtle driver and silicon bugs that are often present with I2C multimaster. > + > + > +Algorithm: > + > +The basic algorithm is to assert your line when you want the bus, then make > +sure that the other side doesn't want it also. A detailed explanation is best > +done with an example. > + > +Let's say the AP wants to claim the bus. It: > +1. Asserts AP_CLAIM. > +2. Waits a little bit for the other side to notice (slew time, say 10 > + microseconds). > +3. Checks EC_CLAIM. If this is not asserted then the AP has the bus and we are > + done. > +4. Otherwise, wait for a few milliseconds and see if EC_CLAIM is released. > +5. If not, back off, release the claim and wait for a few more milliseconds. > +6. Go back to 1 (until retry time has expired). > + > + > +Required properties: > +- compatible: i2c-arbitrator-cros-ec > +- ap-claim-gpio: The GPIO that we (the AP) use to claim the bus. > +- ec-claim-gpio: The GPIO that the other side (the EC) uses the claim the bus. An array based approach like in the i2c-mux-gpio driver would be more generic. Just mention that the driver only supports 2 entries at the moment. > +- Standard I2C mux properties. See mux.txt in this directory. > +- Single I2C child bus node at reg 0. See mux.txt in this directory. > + > +Optional properties: > +- slew-delay-us: microseconds to wait for a GPIO to go high. Default is 10 us. > +- wait-retry-us: we'll attempt another claim after this many microseconds. > + Default is 3000 us. > +- wait-free-us: we'll give up after this many microseconds. Default is 50000 us. Grant, I assume it is okay to introduce these generic bindings? > diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig > index 0be5b83..ca19378 100644 > --- a/drivers/i2c/muxes/Kconfig > +++ b/drivers/i2c/muxes/Kconfig > @@ -5,6 +5,17 @@ > menu "Multiplexer I2C Chip support" > depends on I2C_MUX > > +config I2C_ARBITRATOR_CROS_EC > + tristate "GPIO-based I2C arbitrator used on exynos5250-snow" > + depends on GENERIC_GPIO && OF > + help > + If you say yes to this option, support will be included for an > + I2C multimaster arbitration scheme using GPIOs that is used in > + the Samsung ARM Chromebook (exynos5250-snow). > + > + This driver can also be built as a module. If so, the module > + will be called i2c-arbitrator-cros-ec. > + This text could be more generic then, too. > +static int i2c_arbitrator_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct device_node *parent_np; > + struct i2c_arbitrator_data *arb; > + enum of_gpio_flags gpio_flags; > + unsigned long out_init; > + int ret; > + > + /* We only support probing from device tree; no platform_data */ > + if (WARN_ON(!np)) > + return -ENODEV; Too much WARN_ON for my taste. Thanks, Wolfram -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt new file mode 100644 index 0000000..1f893e7 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt @@ -0,0 +1,76 @@ +GPIO-based Arbitration used by the ARM Chromebook (exynos5250-snow) +=================================================================== +This uses GPIO lines between the AP (Application Processor) and an attached +EC (Embedded Controller) which both want to talk on the same I2C bus as master. + +The AP and EC each have a 'bus claim' line, which is an output that the +other can see. These are both active low, with pull-ups enabled. + +- AP_CLAIM: output from AP, signalling to the EC that the AP wants the bus +- EC_CLAIM: output from EC, signalling to the AP that the EC wants the bus + +This mechanism is used instead of standard I2C multimaster to avoid some of the +subtle driver and silicon bugs that are often present with I2C multimaster. + + +Algorithm: + +The basic algorithm is to assert your line when you want the bus, then make +sure that the other side doesn't want it also. A detailed explanation is best +done with an example. + +Let's say the AP wants to claim the bus. It: +1. Asserts AP_CLAIM. +2. Waits a little bit for the other side to notice (slew time, say 10 + microseconds). +3. Checks EC_CLAIM. If this is not asserted then the AP has the bus and we are + done. +4. Otherwise, wait for a few milliseconds and see if EC_CLAIM is released. +5. If not, back off, release the claim and wait for a few more milliseconds. +6. Go back to 1 (until retry time has expired). + + +Required properties: +- compatible: i2c-arbitrator-cros-ec +- ap-claim-gpio: The GPIO that we (the AP) use to claim the bus. +- ec-claim-gpio: The GPIO that the other side (the EC) uses the claim the bus. +- Standard I2C mux properties. See mux.txt in this directory. +- Single I2C child bus node at reg 0. See mux.txt in this directory. + +Optional properties: +- slew-delay-us: microseconds to wait for a GPIO to go high. Default is 10 us. +- wait-retry-us: we'll attempt another claim after this many microseconds. + Default is 3000 us. +- wait-free-us: we'll give up after this many microseconds. Default is 50000 us. + + +Example: + i2c@12CA0000 { + compatible = "acme,some-i2c-device"; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c-arbitrator { + compatible = "i2c-arbitrator-cros-ec"; + #address-cells = <1>; + #size-cells = <0>; + + i2c-parent = <&{/i2c@12CA0000}>; + + ap-claim-gpio = <&gpf0 3 1 0x10000 0>; + ec-claim-gpio = <&gpe0 4 0 0x10003 0>; + slew-delay-us = <10>; + wait-retry-us = <3000>; + wait-free-us = <50000>; + + i2c@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + + i2c@52 { + // Normal I2C device + }; + }; + }; diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig index 0be5b83..ca19378 100644 --- a/drivers/i2c/muxes/Kconfig +++ b/drivers/i2c/muxes/Kconfig @@ -5,6 +5,17 @@ menu "Multiplexer I2C Chip support" depends on I2C_MUX +config I2C_ARBITRATOR_CROS_EC + tristate "GPIO-based I2C arbitrator used on exynos5250-snow" + depends on GENERIC_GPIO && OF + help + If you say yes to this option, support will be included for an + I2C multimaster arbitration scheme using GPIOs that is used in + the Samsung ARM Chromebook (exynos5250-snow). + + This driver can also be built as a module. If so, the module + will be called i2c-arbitrator-cros-ec. + config I2C_MUX_GPIO tristate "GPIO-based I2C multiplexer" depends on GENERIC_GPIO diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile index 76da869..e60dcc1 100644 --- a/drivers/i2c/muxes/Makefile +++ b/drivers/i2c/muxes/Makefile @@ -1,6 +1,8 @@ # # Makefile for multiplexer I2C chip drivers. +obj-$(CONFIG_I2C_ARBITRATOR_CROS_EC) += i2c-arbitrator-cros-ec.o + obj-$(CONFIG_I2C_MUX_GPIO) += i2c-mux-gpio.o obj-$(CONFIG_I2C_MUX_PCA9541) += i2c-mux-pca9541.o obj-$(CONFIG_I2C_MUX_PCA954x) += i2c-mux-pca954x.o diff --git a/drivers/i2c/muxes/i2c-arbitrator-cros-ec.c b/drivers/i2c/muxes/i2c-arbitrator-cros-ec.c new file mode 100644 index 0000000..f95b591 --- /dev/null +++ b/drivers/i2c/muxes/i2c-arbitrator-cros-ec.c @@ -0,0 +1,222 @@ +/* + * I2C arbitrator driver for the ARM Chromebook + * + * Copyright (C) 2012 Google, Inc + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/delay.h> +#include <linux/gpio.h> +#include <linux/kernel.h> +#include <linux/i2c.h> +#include <linux/i2c-mux.h> +#include <linux/init.h> +#include <linux/module.h> +#include <linux/of_i2c.h> +#include <linux/of_gpio.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + + +/** + * struct i2c_arbitrator_data - Driver data for I2C arbitrator + * + * @parent: Parent adapter + * @child: Child bus + * @ap_gpio: GPIO we'll use to claim. + * @ap_gpio_release: 0 if active high; 1 if active low; AKA if the GPIO == this + * then consider it released. + * @ec_gpio: GPIO that the other side will use to claim. + * @ec_gpio_release: 0 if active high; 1 if active low; AKA if the GPIO == this + * then consider it released. + * @slew_delay_us: microseconds to wait for a GPIO to go high. + * @wait_retry_us: we'll attempt another claim after this many microseconds. + * @wait_free_us: we'll give up after this many microseconds. + */ + +struct i2c_arbitrator_data { + struct i2c_adapter *parent; + struct i2c_adapter *child; + + int ap_gpio; + int ap_gpio_release; + int ec_gpio; + int ec_gpio_release; + unsigned int slew_delay_us; + unsigned int wait_retry_us; + unsigned int wait_free_us; +}; + + +/** + * i2c_arbitrator_select - claim the I2C bus + * + * Use the GPIO-based signalling protocol; return -EBUSY if we fail. + */ +static int i2c_arbitrator_select(struct i2c_adapter *adap, void *data, u32 chan) +{ + const struct i2c_arbitrator_data *arb = data; + unsigned long stop_retry, stop_time; + + /* Start a round of trying to claim the bus */ + stop_time = jiffies + usecs_to_jiffies(arb->wait_free_us) + 1; + do { + /* Indicate that we want to claim the bus */ + gpio_set_value(arb->ap_gpio, !arb->ap_gpio_release); + udelay(arb->slew_delay_us); + + /* Wait for the EC to release it */ + stop_retry = jiffies + usecs_to_jiffies(arb->wait_retry_us) + 1; + while (time_before(jiffies, stop_retry)) { + int gpio_val = !!gpio_get_value(arb->ec_gpio); + + if (gpio_val == arb->ec_gpio_release) { + /* We got it, so return */ + return 0; + } + + usleep_range(50, 200); + } + + /* It didn't release, so give up, wait, and try again */ + gpio_set_value(arb->ap_gpio, arb->ap_gpio_release); + + usleep_range(arb->wait_retry_us, arb->wait_retry_us * 2); + } while (time_before(jiffies, stop_time)); + + /* Give up, release our claim */ + gpio_set_value(arb->ap_gpio, arb->ap_gpio_release); + udelay(arb->slew_delay_us); + dev_err(&adap->dev, "Could not claim bus, timeout\n"); + return -EBUSY; +} + +/** + * i2c_arbitrator_deselect - release the I2C bus + * + * Release the I2C bus using the GPIO-based signalling protocol. + */ +static int i2c_arbitrator_deselect(struct i2c_adapter *adap, void *data, + u32 chan) +{ + const struct i2c_arbitrator_data *arb = data; + + /* Release the bus and wait for the EC to notice */ + gpio_set_value(arb->ap_gpio, arb->ap_gpio_release); + udelay(arb->slew_delay_us); + + return 0; +} + +static int i2c_arbitrator_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct device_node *parent_np; + struct i2c_arbitrator_data *arb; + enum of_gpio_flags gpio_flags; + unsigned long out_init; + int ret; + + /* We only support probing from device tree; no platform_data */ + if (WARN_ON(!np)) + return -ENODEV; + if (WARN_ON(pdev->dev.platform_data)) + return -EINVAL; + + arb = devm_kzalloc(&pdev->dev, sizeof(*arb), GFP_KERNEL); + if (WARN_ON(!arb)) + return -ENOMEM; + platform_set_drvdata(pdev, arb); + + /* Request GPIOs */ + ret = of_get_named_gpio_flags(np, "ap-claim-gpio", 0, &gpio_flags); + if (ret == -EPROBE_DEFER || WARN_ON(!gpio_is_valid(ret))) + return ret; + arb->ap_gpio = ret; + arb->ap_gpio_release = !!(gpio_flags & OF_GPIO_ACTIVE_LOW); + out_init = (gpio_flags & OF_GPIO_ACTIVE_LOW) ? + GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW; + ret = devm_gpio_request_one(&pdev->dev, arb->ap_gpio, out_init, + "ap-claim-gpio"); + if (ret == -EPROBE_DEFER || WARN_ON(ret)) + return ret; + + ret = of_get_named_gpio_flags(np, "ec-claim-gpio", 0, &gpio_flags); + if (ret == -EPROBE_DEFER || WARN_ON(!gpio_is_valid(ret))) + return ret; + arb->ec_gpio = ret; + arb->ec_gpio_release = !!(gpio_flags & OF_GPIO_ACTIVE_LOW); + ret = devm_gpio_request_one(&pdev->dev, arb->ec_gpio, GPIOF_IN, + "ec-claim-gpio"); + if (ret == -EPROBE_DEFER || WARN_ON(ret)) + return ret; + + /* Arbitration parameters */ + if (of_property_read_u32(np, "slew-delay-us", &arb->slew_delay_us)) + arb->slew_delay_us = 10; + if (of_property_read_u32(np, "wait-retry-us", &arb->wait_retry_us)) + arb->wait_retry_us = 3000; + if (of_property_read_u32(np, "wait-free-us", &arb->wait_free_us)) + arb->wait_free_us = 50000; + + /* Find our parent */ + parent_np = of_parse_phandle(np, "i2c-parent", 0); + if (WARN_ON(!parent_np)) + return -EINVAL; + arb->parent = of_find_i2c_adapter_by_node(parent_np); + if (WARN_ON(!arb->parent)) + return -EINVAL; + + /* Actually add the mux adapter */ + arb->child = i2c_add_mux_adapter(arb->parent, &pdev->dev, arb, 0, 0, 0, + i2c_arbitrator_select, + i2c_arbitrator_deselect); + if (WARN_ON(!arb->child)) { + ret = -ENODEV; + i2c_put_adapter(arb->parent); + } + + return ret; +} + +static int i2c_arbitrator_remove(struct platform_device *pdev) +{ + struct i2c_arbitrator_data *arb = platform_get_drvdata(pdev); + + i2c_del_mux_adapter(arb->child); + i2c_put_adapter(arb->parent); + + return 0; +} + +static const struct of_device_id i2c_arbitrator_of_match[] = { + { .compatible = "i2c-arbitrator-cros-ec", }, + {}, +}; +MODULE_DEVICE_TABLE(of, i2c_arbitrator_of_match); + +static struct platform_driver i2c_arbitrator_driver = { + .probe = i2c_arbitrator_probe, + .remove = i2c_arbitrator_remove, + .driver = { + .owner = THIS_MODULE, + .name = "i2c-arbitrator-cros-ec", + .of_match_table = of_match_ptr(i2c_arbitrator_of_match), + }, +}; + +module_platform_driver(i2c_arbitrator_driver); + +MODULE_DESCRIPTION("I2C arbitrator driver for the ARM Chromebook"); +MODULE_AUTHOR("Doug Anderson <dianders@chromium.org>"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:i2c-arbitrator-cros-ec");