Stable I2C branch needed as a dependency for Tegra change
Wolfram, A change I'd like to make for Tegra in 3,5: http://www.spinics.net/lists/arm-kernel/msg172756.html relies on an I2C change that's also scheduled for 3.5: http://www.spinics.net/lists/linux-i2c/msg07808.html (which is already committed as 1475e39 "of/i2c: implement of_find_i2c_adapter_by_node") Is your i2c-embedded/for-next branch stable (i.e. that commit will not be rebased between now and when you send a pull request for 3.5), so I can use it as a baseline for a Tegra branch? If not, could you let me know when it is stable, so I can create the Tegra branch as that time. Thanks! -- 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 5/5] drm: Add NVIDIA Tegra support
On 04/25/2012 03:45 AM, Thierry Reding wrote:
> This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It
> currently has rudimentary GEM support and can run a console on the
> framebuffer as well as X using the xf86-video-modesetting driver. Only
> the RGB output is supported.
>
> HDMI support was taken from NVIDIA's Linux kernel tree but it doesn't
> quite work. EDID data can be retrieved but the output doesn't properly
> activate the connected TV.
>
> The DSI and TVO outputs and the HOST1X driver are just stubs that setup
> the corresponding resources but don't do anything useful yet.
I'm mainly going to comment on the device tree bindings here; hopefully
Jon and Terje can chime in on the code itself.
> diff --git a/Documentation/devicetree/bindings/gpu/drm/tegra.txt
> b/Documentation/devicetree/bindings/gpu/drm/tegra.txt
> +Example:
> +
> +/memreserve/ 0x0e00 0x0200;
> +
> +...
> +
> +/ {
> + ...
> +
> + /* host1x */
> + host1x: host1x@5000 {
> + compatible = "nvidia,tegra20-host1x";
> + reg = <0x5000 0x00024000>;
> + interrupts = <0 64 0x04 /* cop syncpt */
> + 0 65 0x04 /* mpcore syncpt */
> + 0 66 0x04 /* cop general */
> + 0 67 0x04>; /* mpcore general */
> + };
The host1x module is apparently a register bus, behind which all the
other modules sit. According to the address map in the TRM, "all the
other modules" appears to include all of MPE, VI, CSI, EPP, ISP, GR2D,
GR3D, DISPLAY A/B, HDMI, TVO, DSI, plus VG, VS, VCI, DSIB on Tegra30.
That said, I believe Terje wasn't convinced that all those modules are
really host1x children, just some. Can you check please, Terje?
Also, I wonder if host1x is really the parent of these modules,
register-bus-access-wise, or whether the bus covers the "graphic host
registers" at 0x5000-0x50023fff?
As such, I think the DT nodes for all those modules should be within the
host1x node (or perhaps graphics host node, pending above discussion):
host1x: host1x@5000 {
mpe@5404 { ... };
vi@5408 { ... };
...
};
host1x can easily instantiate all the child nodes simply by calling
of_platform_populate() at the end of its probe; see
sound/soc/tegra/tegra30_ahub.c for an example.
> + /* video-encoding/decoding */
> + mpe@5404 {
> + reg = <0x5404 0x0004>;
> + interrupts = <0 68 0x04>;
> + };
We'll probably end up needing a phandle from each of these nodes to
host1x, and a channel ID, so the drivers for these nodes can register
themselves with host1x. However, I it's probably OK to defer the DT
binding for this until we actually start working on command-channels.
> + /* graphics host */
> + graphics@5400 {
> + compatible = "nvidia,tegra20-graphics";
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
I don't think those 3 properties are needed, unless there are child
nodes with registers.
> + display-controllers = <&disp1 &disp2>;
> + carveout = <0x0e00 0x0200>;
> + host1x = <&host1x>;
> + gart = <&gart>;
> +
> + connectors {
I'm not sure that it makes sense to put the connectors nodes underneath
the graphics host node; the connectors aren't objects with registers
that are accessed through a bus managed by the graphics node. Equally,
the connectors could be useful outside of the graphics driver - e.g. the
audio driver might need to refer to an HDMI connector.
Instead, I'd probably put the connector nodes at the top level of the
device tree, and have graphics contain a property that lists the
phandles of all available connectors.
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + connector@0 {
> + reg = <0>;
> + edid = /incbin/("machine.edid");
> + output = <&lvds>;
> + };
I think each of these needs some kind of compatible value to indicate
their type. Also, the ability to represent both HDMI video and audio
streams so that a sound card binding could use the HDMI connector too. I
see that drivers/extcon/ has appeared recently, and I wonder if the
connector nodes in DT shouldn't be handled by that subsystem?
> + connector@1 {
> + reg = <1>;
> + output = <&hdmi>;
> + ddc = <&i2c2>;
> +
> + hpd-gpio = <&gpio 111 0>; /* PN7 */
> + };
> + };
> + };
> +};
> diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c
> b/arch/arm/mach-tegra/board-dt-tegra20.c
> + { "host1x", "pll_c",14400, true },
> + { "disp1", "pl
Re: [PATCH RESEND] i2c/nomadik: runtime PM support
On Thursday, May 03, 2012, Mark Brown wrote: > On Thu, May 03, 2012 at 02:59:25PM +0200, Rafael J. Wysocki wrote: > > On Thursday, May 03, 2012, Linus Walleij wrote: > > > > If *all* runtime_resume() hooks for *all* devices in the same power > > > domain are called at this time on the way up/down we get a major > > > overhead as our primary power domain is pretty big. > > > FWIW, in the generic PM domains framework used on the sh7372 platform we > > have > > a need_restore flag whose meaning is whether or not to call > > .runtime_suspend() > > (or its domain-specific counterpart) when the domain is about to be powered > > off and .runtime_resume() when the device is resumed by the runtime PM > > framework. Those two callbacks are meant to save and restore the device's > > state, respectively, and there are two more domain-specific callbacks, > > .stop() and .start() that (in the case of sh7372) manipiulate the devices' > > clocks. > > > So, if pm_runtime_suspend() is called for a device, we first check if it's > > OK > > to call .stop() for it (according to PM QoS constraints), but we don't call > > .runtime_suspend() for it yet at this point. Next, we check if it's OK to > > power off the domain containing the device (taking PM QoS constraints and > > subdomains into consideration) and if we decide to do that, we call > > .runtime_suspend() for the devices in the domain, but only those whose > > need_restore flags are unset. Then, if pm_runtime_resume() is called for > > one of the devices, we check its need_restore flag and call > > .runtime_resume() > > for the device if the flag set and .start() is called subsequently. > > This seems like a really useful idiom in general - might it be worth > supporting as a standard framework feature? The generic PM domains framework does this, it's not platform-specific (if I understood your question correctly). Thanks, Rafael -- 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: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
Hi Mark,
On Thu, 3 May 2012 11:53:36 +0100, Mark Brown wrote:
> Since there are uses for I2C_M_NOSTART which are much more sensible and
> standard than most of the protocol mangling functionality (the main one
> being gather writes to devices where something like a register address
> needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> for this feature and update all the users to use it.
This is all correct, but it should be documented in
Documentation/i2c/i2c-protocol. At the moment documentation still says
that I2C_M_NOSTART is a weird protocol quirk nobody should be using.
When you update the documentation, I think it is important to stress
that there are now two use cases of I2C_M_NOSTART. If direction
changes, it is a rarely needed protocol quirk. If direction doesn't
change, it is used for buffer gathering.
>
> In the case of regmap-i2c we remove the requirement for mangling as
> I2C_M_NOSTART is the only mangling feature which is being used.
>
> Signed-off-by: Mark Brown
> ---
> Documentation/i2c/functionality |1 +
> drivers/base/regmap/regmap-i2c.c |2 +-
> drivers/i2c/algos/i2c-algo-bit.c |2 +-
> drivers/i2c/algos/i2c-algo-pcf.c |2 +-
> drivers/i2c/busses/i2c-nuc900.c |3 ++-
> drivers/i2c/busses/i2c-s3c2410.c |3 ++-
> drivers/input/joystick/as5011.c |1 +
> drivers/video/matrox/matroxfb_maven.c |1 +
> include/linux/i2c.h |5 +++--
> 9 files changed, 13 insertions(+), 7 deletions(-)
Review:
>
> diff --git a/Documentation/i2c/functionality b/Documentation/i2c/functionality
> index 42c17c1..fb2e77e 100644
> --- a/Documentation/i2c/functionality
> +++ b/Documentation/i2c/functionality
> @@ -33,6 +33,7 @@ For the most up-to-date list of functionality constants,
> please check
You must also update the description of I2C_FUNC_PROTOCOL_MANGLING to
no longer mention I2C_M_NOSTART.
>I2C_FUNC_SMBUS_WRITE_BLOCK_DATA Handles the SMBus write_block_data command
>I2C_FUNC_SMBUS_READ_I2C_BLOCK Handles the SMBus read_i2c_block_data
> command
>I2C_FUNC_SMBUS_WRITE_I2C_BLOCK Handles the SMBus write_i2c_block_data
> command
> + I2C_FUNC_NOSTARTTransfers can be sent without a start
Not true. Transfers always begin with a Start, even when I2C_M_NOSTART
is used, because it is never set for the first message.
I2C_FUNC_NOSTART means that messages other than the first may not get
their usual Repeated Start and address prefix. So a better working IMHO
would be:
+ I2C_FUNC_NOSTARTCan skip repeated start sequence
>
> A few combinations of the above flags are also defined for your convenience:
>
> diff --git a/drivers/base/regmap/regmap-i2c.c
> b/drivers/base/regmap/regmap-i2c.c
> index 5f6b247..fa6bf52 100644
> --- a/drivers/base/regmap/regmap-i2c.c
> +++ b/drivers/base/regmap/regmap-i2c.c
> @@ -42,7 +42,7 @@ static int regmap_i2c_gather_write(void *context,
> /* If the I2C controller can't do a gather tell the core, it
>* will substitute in a linear write for us.
>*/
> - if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
> + if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_NOSTART))
> return -ENOTSUPP;
>
> xfer[0].addr = i2c->addr;
> diff --git a/drivers/i2c/algos/i2c-algo-bit.c
> b/drivers/i2c/algos/i2c-algo-bit.c
> index 7f0b832..fad22b0 100644
> --- a/drivers/i2c/algos/i2c-algo-bit.c
> +++ b/drivers/i2c/algos/i2c-algo-bit.c
> @@ -608,7 +608,7 @@ bailout:
>
> static u32 bit_func(struct i2c_adapter *adap)
> {
> - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> + return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
> I2C_FUNC_SMBUS_READ_BLOCK_DATA |
> I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
> I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
> diff --git a/drivers/i2c/algos/i2c-algo-pcf.c
> b/drivers/i2c/algos/i2c-algo-pcf.c
> index 5c23795..8b38986 100644
> --- a/drivers/i2c/algos/i2c-algo-pcf.c
> +++ b/drivers/i2c/algos/i2c-algo-pcf.c
> @@ -401,7 +401,7 @@ out:
>
> static u32 pcf_func(struct i2c_adapter *adap)
> {
> - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> + return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
> I2C_FUNC_PROTOCOL_MANGLING;
> }
>
A quick grep suggests that i2c-algo-pcf doesn't support I2C_M_NOSTART,
only I2C_M_REV_DIR_ADDR. So you don't want to add I2C_FUNC_NOSTART to
the functionality mask.
> diff --git a/drivers/i2c/busses/i2c-nuc900.c b/drivers/i2c/busses/i2c-nuc900.c
> index 03b6157..a26dfb8 100644
> --- a/drivers/i2c/busses/i2c-nuc900.c
> +++ b/drivers/i2c/busses/i2c-nuc900.c
> @@ -502,7 +502,8 @@ static int nuc900_i2c_xfer(struct i2c_adapter *adap,
> /* declare our i2c functionality */
> static u32 nuc900_i2c_func(struct i2c_adapter *adap)
> {
> - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
> + re
Re: [PATCH] I2C: xiic: Add OF binding support
On 05/03/2012 01:10 PM, Wolfram Sang wrote: > On Wed, Apr 25, 2012 at 03:48:53PM +0200, Lars-Peter Clausen wrote: >> Signed-off-by: Lars-Peter Clausen > > Applied to next, thanks. > > How are your experiences with the driver? Does it still need > EXPERIMENTAL? I had some issues under certain conditions, e.g. certain types of I2C transfers would confuse the core and fail. But I hadn't had the time yet to investigate further. So I wouldn't remove the EXPERIMENTAL yet. - Lars -- 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] i2c: davinci: Free requested IRQ in remove
The freed IRQ is not necessary the one requested in probe. Even if it was, with two or more i2c-controllers it will fails anyway. Signed-off-by: Marcus Folkesson --- drivers/i2c/busses/i2c-davinci.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c index a76d85f..79b4bcb 100644 --- a/drivers/i2c/busses/i2c-davinci.c +++ b/drivers/i2c/busses/i2c-davinci.c @@ -755,7 +755,7 @@ static int davinci_i2c_remove(struct platform_device *pdev) dev->clk = NULL; davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0); - free_irq(IRQ_I2C, dev); + free_irq(dev->irq, dev); iounmap(dev->base); kfree(dev); -- 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 RESEND] i2c/nomadik: runtime PM support
On Thu, May 03, 2012 at 02:59:25PM +0200, Rafael J. Wysocki wrote: > On Thursday, May 03, 2012, Linus Walleij wrote: > > If *all* runtime_resume() hooks for *all* devices in the same power > > domain are called at this time on the way up/down we get a major > > overhead as our primary power domain is pretty big. > FWIW, in the generic PM domains framework used on the sh7372 platform we have > a need_restore flag whose meaning is whether or not to call .runtime_suspend() > (or its domain-specific counterpart) when the domain is about to be powered > off and .runtime_resume() when the device is resumed by the runtime PM > framework. Those two callbacks are meant to save and restore the device's > state, respectively, and there are two more domain-specific callbacks, > .stop() and .start() that (in the case of sh7372) manipiulate the devices' > clocks. > So, if pm_runtime_suspend() is called for a device, we first check if it's OK > to call .stop() for it (according to PM QoS constraints), but we don't call > .runtime_suspend() for it yet at this point. Next, we check if it's OK to > power off the domain containing the device (taking PM QoS constraints and > subdomains into consideration) and if we decide to do that, we call > .runtime_suspend() for the devices in the domain, but only those whose > need_restore flags are unset. Then, if pm_runtime_resume() is called for > one of the devices, we check its need_restore flag and call .runtime_resume() > for the device if the flag set and .start() is called subsequently. This seems like a really useful idiom in general - might it be worth supporting as a standard framework feature? signature.asc Description: Digital signature
Re: [PATCH RESEND] i2c/nomadik: runtime PM support
On Thursday, May 03, 2012, Linus Walleij wrote: > On Wed, Apr 18, 2012 at 8:39 AM, Magnus Damm wrote: > > > I'm a bit hesitant to ack because the runtime suspend/resume callbacks > > are used differently than we would do on arch/sh and > > arch/arm/mach-shmobile. This difference may or may not be a good > > thing. I'm afraid I know too little about the nomadik platform to say > > anything wise. =) > > > > Our drivers assume that the ->runtime_suspend() and ->runtime_resume() > > callbacks are executed before/after the power is turned off/on for a > > certain power domain. The way they are used in this patch more seems > > like they are expected to be called regardless of the state of the > > rest of the devices sharing the underlying power domain. > > After discussing this internally a bit I think this is the central point here, > and yes that is indeed how it is written. > > We are not using runtime_[suspend|resume]() like that but want > this to be handled on each and every driver in isolation. > > So our handler kicks in at the pm_domain level, then calls the > driver hook from the domain by a simple > pm_generic_runtime_suspend(dev) *every* time, not just > if the power domain was switched on/off. > > The reason is mainly that we want register save/restore to be done > as soon as a driver needs it, not before/after its power domain is cut > as mentioned here. > > Example: the system is sleeping. We need to wake up and do > some sporadic task in one driver, then go back to sleep. > > If *all* runtime_resume() hooks for *all* devices in the same power > domain are called at this time on the way up/down we get a major > overhead as our primary power domain is pretty big. FWIW, in the generic PM domains framework used on the sh7372 platform we have a need_restore flag whose meaning is whether or not to call .runtime_suspend() (or its domain-specific counterpart) when the domain is about to be powered off and .runtime_resume() when the device is resumed by the runtime PM framework. Those two callbacks are meant to save and restore the device's state, respectively, and there are two more domain-specific callbacks, .stop() and .start() that (in the case of sh7372) manipiulate the devices' clocks. So, if pm_runtime_suspend() is called for a device, we first check if it's OK to call .stop() for it (according to PM QoS constraints), but we don't call .runtime_suspend() for it yet at this point. Next, we check if it's OK to power off the domain containing the device (taking PM QoS constraints and subdomains into consideration) and if we decide to do that, we call .runtime_suspend() for the devices in the domain, but only those whose need_restore flags are unset. Then, if pm_runtime_resume() is called for one of the devices, we check its need_restore flag and call .runtime_resume() for the device if the flag set and .start() is called subsequently. > We prefer to be able to control on a per-driver instance how this > is handled. That may include e.g. clock handling as part of the > pm_runtime hooks, but not necessarily, register save/restore is > the big bulk of work. > > Another thing that sort of mandates this approach is that we > have drivers that are used on multiple systems beside ux500. > E.g the UART PL011 is used on a plethora of systems. > > Some of these may use the bus hooks (or type, class...) > calling into the drivers runtime_pm() hooks, if we start to > encode semantics into the driver regarding how these hooks > get called or assume they are always called from a power > domain we're inviting disaster I'm afraid :-/ The generic PM domains framework allows you to use different callbacks with domains in general (ie. it is possible to use two different sets of PM callbacks depending on whether the device is in a domain or not). There may be a couple of things still missing depending on the use case, but the general support is there. > On systems where we know the driver is always used with > power domains the world is easier, but this is not the case > here. This piece of code in rumtime PM is the source of the > confusion: > > if (dev->pm_domain) > callback = dev->pm_domain->ops.runtime_suspend; > else if (dev->type && dev->type->pm) > callback = dev->type->pm->runtime_suspend; > else if (dev->class && dev->class->pm) > callback = dev->class->pm->runtime_suspend; > else if (dev->bus && dev->bus->pm) > callback = dev->bus->pm->runtime_suspend; > else > callback = NULL; > > if (!callback && dev->driver && dev->driver->pm) > callback = dev->driver->pm->runtime_suspend; > > So for a particular driver we don't know which one it's > going to be. This doesn't seem to be particularly difficult to figure out in advance, though. The diffucult part is to know what the domain/subsystem will do in addition to executing your callbacks, but the gener
[PULL REQUEST] i2c-embedded for 3.4
Linus, here are some typical i2c driver bugfixes for 3.4. Missed clock handling, improper timeout fixes, hardware wrokarounds... All patches have been in linux-next for a few days, too. Please pull. Thanks, Wolfram The following changes since commit 66f75a5d028beaf67c931435fdc3e7823125730c: Linux 3.4-rc4 (2012-04-21 14:47:52 -0700) are available in the git repository at: git://git.pengutronix.de/git/wsa/linux.git i2c-embedded/for-current for you to fetch changes up to 1e4f0b82577e59f23484c99056c96465e202fdd5: i2c: mxs: disable QUEUE when sending is done (2012-04-27 16:13:29 +0200) Alok Chauhan (1): i2c: tegra: Add delay before resetting the controller after NACK Roland Stigge (1): i2c: pnx: Disable clk in suspend Tomoya MORINAGA (2): i2c-eg20t: change timeout value 50msec to 1000msec i2c-eg20t: Modify MODULE_AUTHOR's email address Wolfram Sang (2): i2c: mxs: handle spurious interrupt i2c: mxs: disable QUEUE when sending is done drivers/i2c/busses/i2c-eg20t.c |4 ++-- drivers/i2c/busses/i2c-mxs.c |8 drivers/i2c/busses/i2c-pnx.c |3 +-- drivers/i2c/busses/i2c-tegra.c |8 4 files changed, 15 insertions(+), 8 deletions(-) -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH V3] i2c: Add generic I2C multiplexer using pinctrl API
On Tue, May 01, 2012 at 11:23:31AM -0600, Stephen Warren wrote: > From: Stephen Warren > > 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 | > +-+ +-+ +-+ > > Signed-off-by: Stephen Warren > Acked-by: Linus Walleij > --- > v3: Renamed pinctrl-i2cmux.c to i2c-mux-pinctrl.c to match recent changes > to other I2C mux files. Thanks for doing the rename already. While I think I could follow your argument regarding the compatible-binding, I'd still like to have an ack from one of the device tree maintainers. The bindings are non-trivial and I am missing the insight to judge them. Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
On Thu, May 03, 2012 at 11:53:36AM +0100, Mark Brown wrote: > Since there are uses for I2C_M_NOSTART which are much more sensible and > standard than most of the protocol mangling functionality (the main one > being gather writes to devices where something like a register address > needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART > for this feature and update all the users to use it. > > In the case of regmap-i2c we remove the requirement for mangling as > I2C_M_NOSTART is the only mangling feature which is being used. > > Signed-off-by: Mark Brown Applied to next, thanks! Jean, let me know if you prefer to take it. Acks from input and fbdev maintainers still appreciated. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2 1/5] i2c: Convert i2c-octeon.c to use device tree.
On Thu, Apr 26, 2012 at 06:20:26PM -0700, David Daney wrote: > From: David Daney > > 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. > > Signed-off-by: David Daney > Acked-by: Rob Herring > Cc: "Jean Delvare (PC drivers, core)" > Cc: "Ben Dooks (embedded platforms)" > Cc: "Wolfram Sang (embedded platforms)" > Cc: linux-i2c@vger.kernel.org I2C changes look okay. I'd think this goes in via MIPS-tree? Acked-by: Wolfram Sang -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH RESEND] i2c/nomadik: runtime PM support
On Thu, May 03, 2012 at 11:03:12AM +0200, Linus Walleij wrote: > On Wed, Apr 18, 2012 at 8:39 AM, Magnus Damm wrote: > > Our drivers assume that the ->runtime_suspend() and ->runtime_resume() > > callbacks are executed before/after the power is turned off/on for a > > certain power domain. The way they are used in this patch more seems > > like they are expected to be called regardless of the state of the > > rest of the devices sharing the underlying power domain. ... > On systems where we know the driver is always used with > power domains the world is easier, but this is not the case > here. This piece of code in rumtime PM is the source of the > confusion: One thing that the regulator framework has which might be useful here is that it can notify drivers when power is actually removed. This allows drivers to reduce the amount of work they need to do to bring the device back up if they know power was retained while they were suspended. It feels like some of the confusion is over the difference between quiescing the device when idle and actually removing power from the device. signature.asc Description: Digital signature
Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
On Thu, May 03, 2012 at 10:58:15AM +0100, Mark Brown wrote: > On Thu, May 03, 2012 at 11:52:11AM +0200, Wolfram Sang wrote: > > > Also, I'd think the FUNC_NOSTART bit should be 0x08 and SMBUS_PEC 0x4000. > > This > > will be more intuitive, probably? > > What is the value in renumbering everything? It just seems like it > makes the diff less clear and has no practical value. Not everything, only those 2. The result would be having one block dealing with I2C and one block with SMBUS. But Jean's comment is an ultimate "no" anyway. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] I2C: xiic: Add OF binding support
On Wed, Apr 25, 2012 at 03:48:53PM +0200, Lars-Peter Clausen wrote: > Signed-off-by: Lars-Peter Clausen Applied to next, thanks. How are your experiences with the driver? Does it still need EXPERIMENTAL? -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
On Thu, 3 May 2012 10:58:15 +0100, Mark Brown wrote: > On Thu, May 03, 2012 at 11:52:11AM +0200, Wolfram Sang wrote: > > > Also, I'd think the FUNC_NOSTART bit should be 0x08 and SMBUS_PEC 0x4000. > > This > > will be more intuitive, probably? > > What is the value in renumbering everything? It just seems like it > makes the diff less clear and has no practical value. You actually just can't renumber them, as they are part of the ABI to user-space (through i2c-dev's ioctl I2C_FUNCS.) -- Jean Delvare -- 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] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
Since there are uses for I2C_M_NOSTART which are much more sensible and
standard than most of the protocol mangling functionality (the main one
being gather writes to devices where something like a register address
needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
for this feature and update all the users to use it.
In the case of regmap-i2c we remove the requirement for mangling as
I2C_M_NOSTART is the only mangling feature which is being used.
Signed-off-by: Mark Brown
---
Documentation/i2c/functionality |1 +
drivers/base/regmap/regmap-i2c.c |2 +-
drivers/i2c/algos/i2c-algo-bit.c |2 +-
drivers/i2c/algos/i2c-algo-pcf.c |2 +-
drivers/i2c/busses/i2c-nuc900.c |3 ++-
drivers/i2c/busses/i2c-s3c2410.c |3 ++-
drivers/input/joystick/as5011.c |1 +
drivers/video/matrox/matroxfb_maven.c |1 +
include/linux/i2c.h |5 +++--
9 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/Documentation/i2c/functionality b/Documentation/i2c/functionality
index 42c17c1..fb2e77e 100644
--- a/Documentation/i2c/functionality
+++ b/Documentation/i2c/functionality
@@ -33,6 +33,7 @@ For the most up-to-date list of functionality constants,
please check
I2C_FUNC_SMBUS_WRITE_BLOCK_DATA Handles the SMBus write_block_data command
I2C_FUNC_SMBUS_READ_I2C_BLOCK Handles the SMBus read_i2c_block_data command
I2C_FUNC_SMBUS_WRITE_I2C_BLOCK Handles the SMBus write_i2c_block_data
command
+ I2C_FUNC_NOSTARTTransfers can be sent without a start
A few combinations of the above flags are also defined for your convenience:
diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 5f6b247..fa6bf52 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -42,7 +42,7 @@ static int regmap_i2c_gather_write(void *context,
/* If the I2C controller can't do a gather tell the core, it
* will substitute in a linear write for us.
*/
- if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
+ if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_NOSTART))
return -ENOTSUPP;
xfer[0].addr = i2c->addr;
diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c
index 7f0b832..fad22b0 100644
--- a/drivers/i2c/algos/i2c-algo-bit.c
+++ b/drivers/i2c/algos/i2c-algo-bit.c
@@ -608,7 +608,7 @@ bailout:
static u32 bit_func(struct i2c_adapter *adap)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+ return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
I2C_FUNC_SMBUS_READ_BLOCK_DATA |
I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index 5c23795..8b38986 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -401,7 +401,7 @@ out:
static u32 pcf_func(struct i2c_adapter *adap)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+ return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
I2C_FUNC_PROTOCOL_MANGLING;
}
diff --git a/drivers/i2c/busses/i2c-nuc900.c b/drivers/i2c/busses/i2c-nuc900.c
index 03b6157..a26dfb8 100644
--- a/drivers/i2c/busses/i2c-nuc900.c
+++ b/drivers/i2c/busses/i2c-nuc900.c
@@ -502,7 +502,8 @@ static int nuc900_i2c_xfer(struct i2c_adapter *adap,
/* declare our i2c functionality */
static u32 nuc900_i2c_func(struct i2c_adapter *adap)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
+ I2C_FUNC_PROTOCOL_MANGLING;
}
/* i2c bus registration info */
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index fa0b134..0195915 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -626,7 +626,8 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
/* declare our i2c functionality */
static u32 s3c24xx_i2c_func(struct i2c_adapter *adap)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
+ I2C_FUNC_PROTOCOL_MANGLING;
}
/* i2c bus registration info */
diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
index 3063464..57d19d4 100644
--- a/drivers/input/joystick/as5011.c
+++ b/drivers/input/joystick/as5011.c
@@ -231,6 +231,7 @@ static int __devinit as5011_probe(struct i2c_client *client,
}
if (!i2c_check_functionality(client->adapter,
+I2C_FUNC_NOSTART |
I2C_FUNC_PROTOCOL_MANGLING)) {
dev_err(&client->dev,
"need i2c bus that supports p
Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
On Thu, May 03, 2012 at 11:52:11AM +0200, Wolfram Sang wrote: > Also, I'd think the FUNC_NOSTART bit should be 0x08 and SMBUS_PEC 0x4000. This > will be more intuitive, probably? What is the value in renumbering everything? It just seems like it makes the diff less clear and has no practical value. signature.asc Description: Digital signature
Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
On Thu, Apr 26, 2012 at 01:37:19PM +0100, Mark Brown wrote:
> Since there are uses for I2C_M_NOSTART which are much more sensible and
> standard than most of the protocol mangling functionality (the main one
> being gather writes to devices where something like a register address
> needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> for this feature and update all the users to use it.
>
> In the case of regmap-i2c we remove the requirement for mangling as
> I2C_M_NOSTART is the only mangling feature which is being used.
>
> Signed-off-by: Mark Brown
In general, I like it. One thing below:
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -540,7 +540,7 @@ struct i2c_msg {
> __u16 flags;
> #define I2C_M_TEN0x0010 /* this is a ten bit chip address */
> #define I2C_M_RD 0x0001 /* read data, from slave to master */
> -#define I2C_M_NOSTART0x4000 /* if
> I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_NOSTART0x4000 /* if I2C_FUNC_NOSTART */
> #define I2C_M_REV_DIR_ADDR 0x2000 /* if I2C_FUNC_PROTOCOL_MANGLING */
> #define I2C_M_IGNORE_NAK 0x1000 /* if I2C_FUNC_PROTOCOL_MANGLING */
> #define I2C_M_NO_RD_ACK 0x0800 /* if
> I2C_FUNC_PROTOCOL_MANGLING */
> @@ -568,6 +568,7 @@ struct i2c_msg {
> #define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x0200
> #define I2C_FUNC_SMBUS_READ_I2C_BLOCK0x0400 /* I2C-like block
> xfer */
> #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK 0x0800 /* w/ 1-byte reg.
> addr. */
> +#define I2C_FUNC_NOSTART 0x1000 /* I2C_M_NOSTART */
In this file, the comment for MANGLING should be adapted, too. It says
currently:
#define I2C_FUNC_PROTOCOL_MANGLING 0x0004 /* I2C_M_NOSTART etc. */
NOSTART is now out of MANGLING.
Also, I'd think the FUNC_NOSTART bit should be 0x08 and SMBUS_PEC 0x4000. This
will be more intuitive, probably?
Thanks,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 2/2] i2c-mux-gpio: Rename platform data structure
On Sat, Apr 28, 2012 at 03:35:51PM +0200, Jean Delvare wrote: > Align the name of i2c-mux-gpio's data structure on the new driver name. > Also change one define and adjust function names, even if they aren't > part of the public interface, for consistency. > > Signed-off-by: Jean Delvare > Cc: Peter Korsgaard > Cc: Wolfram Sang > --- > I made this a separate patch for easier testing/review/bisection. > Wolfram, feel free to merge into the first patch if you prefer a > single patch. Applied both, squashed them (was still readable IMO) and fixed it up because of the recent i2c_add_mux_adapter() rework. Thanks a lot! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH RESEND] i2c/nomadik: runtime PM support
On Wed, Apr 18, 2012 at 8:39 AM, Magnus Damm wrote:
> I'm a bit hesitant to ack because the runtime suspend/resume callbacks
> are used differently than we would do on arch/sh and
> arch/arm/mach-shmobile. This difference may or may not be a good
> thing. I'm afraid I know too little about the nomadik platform to say
> anything wise. =)
>
> Our drivers assume that the ->runtime_suspend() and ->runtime_resume()
> callbacks are executed before/after the power is turned off/on for a
> certain power domain. The way they are used in this patch more seems
> like they are expected to be called regardless of the state of the
> rest of the devices sharing the underlying power domain.
After discussing this internally a bit I think this is the central point here,
and yes that is indeed how it is written.
We are not using runtime_[suspend|resume]() like that but want
this to be handled on each and every driver in isolation.
So our handler kicks in at the pm_domain level, then calls the
driver hook from the domain by a simple
pm_generic_runtime_suspend(dev) *every* time, not just
if the power domain was switched on/off.
The reason is mainly that we want register save/restore to be done
as soon as a driver needs it, not before/after its power domain is cut
as mentioned here.
Example: the system is sleeping. We need to wake up and do
some sporadic task in one driver, then go back to sleep.
If *all* runtime_resume() hooks for *all* devices in the same power
domain are called at this time on the way up/down we get a major
overhead as our primary power domain is pretty big.
We prefer to be able to control on a per-driver instance how this
is handled. That may include e.g. clock handling as part of the
pm_runtime hooks, but not necessarily, register save/restore is
the big bulk of work.
Another thing that sort of mandates this approach is that we
have drivers that are used on multiple systems beside ux500.
E.g the UART PL011 is used on a plethora of systems.
Some of these may use the bus hooks (or type, class...)
calling into the drivers runtime_pm() hooks, if we start to
encode semantics into the driver regarding how these hooks
get called or assume they are always called from a power
domain we're inviting disaster I'm afraid :-/
On systems where we know the driver is always used with
power domains the world is easier, but this is not the case
here. This piece of code in rumtime PM is the source of the
confusion:
if (dev->pm_domain)
callback = dev->pm_domain->ops.runtime_suspend;
else if (dev->type && dev->type->pm)
callback = dev->type->pm->runtime_suspend;
else if (dev->class && dev->class->pm)
callback = dev->class->pm->runtime_suspend;
else if (dev->bus && dev->bus->pm)
callback = dev->bus->pm->runtime_suspend;
else
callback = NULL;
if (!callback && dev->driver && dev->driver->pm)
callback = dev->driver->pm->runtime_suspend;
So for a particular driver we don't know which one it's
going to be. But we need to assume they all call down
to the runtime_suspend/resume hooks with something like
pm_generic_runtime_suspend(dev) regardless.
For example the bus code for AMBA does this:
static int amba_pm_runtime_suspend(struct device *dev)
{
struct amba_device *pcdev = to_amba_device(dev);
int ret = pm_generic_runtime_suspend(dev);
if (ret == 0 && dev->driver)
clk_disable(pcdev->pclk);
return ret;
}
Platform devices does this:
static const struct dev_pm_ops platform_dev_pm_ops = {
.runtime_suspend = pm_generic_runtime_suspend,
.runtime_resume = pm_generic_runtime_resume,
.runtime_idle = pm_generic_runtime_idle,
USE_PLATFORM_PM_SLEEP_OPS
};
So ... we assume pm_generic_runtime_suspend() is always
called one way or another. How else can we have code that
work with both bus code like this and out power domains?
I see that shmobile does something totally different, but its
drivers are not used by others are they?
> You probably
> want to control the clocks and the regulators directly - independently
> of the rest of the devices.You may want to look into struct
> gpd_dev_ops for various ways to interface to the pm domain code.
In this specific driver we may want to keep the clock control
in the main code path, and move the regulator, which is actually
a power domain switch, to the power domain (genpd) framework.
But that is not the case with all drivers, so while I can device a
way forward in this case it doesn't play well in the generic sense.
Yours,
Linus Walleij
--
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 V1 1/2] i2c: tegra: fix 10bit address configuration
On Thursday 03 May 2012 01:48 PM, Wolfram Sang wrote: * PGP Signed by an unknown key On Thu, May 03, 2012 at 11:43:14AM +0530, Laxman Dewangan wrote: On Tuesday 24 April 2012 02:28 PM, Jean Delvare wrote: On Tue, 24 Apr 2012 12:49:35 +0530, Laxman Dewangan wrote: The slave address of device to be configured in packet header as follows: 7 bit address: PacketHeader3[7:1] 10 bit address: PacketHeader3[9:0] Fixing the code to make packet header3 properly. Signed-off-by: Laxman Dewangan --- Looks good. Acked-by: Jean Delvare Can it be apply? This is independent of other change in this series which is under discussion/review. Applied to next. Or do you need it in 3.4? I dont need it on 3.4. It is fine to apply for next only. -- 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 V1 1/2] i2c: tegra: fix 10bit address configuration
On Thu, May 03, 2012 at 11:43:14AM +0530, Laxman Dewangan wrote: > On Tuesday 24 April 2012 02:28 PM, Jean Delvare wrote: > >On Tue, 24 Apr 2012 12:49:35 +0530, Laxman Dewangan wrote: > >>The slave address of device to be configured in packet > >>header as follows: > >> 7 bit address: PacketHeader3[7:1] > >> 10 bit address: PacketHeader3[9:0] > >> > >>Fixing the code to make packet header3 properly. > >> > >>Signed-off-by: Laxman Dewangan > >>--- > >> > >Looks good. > > > >Acked-by: Jean Delvare > > > Can it be apply? This is independent of other change in this series > which is under discussion/review. Applied to next. Or do you need it in 3.4? -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
