Re: [PATCH v3 2/2] ACPI / gpio: Add irq_type when a gpio is used as an interrupt
On Mon, Nov 30, 2015 at 11:47 PM, Christophe Ricard <christophe.ric...@gmail.com> wrote: > When a gpio is used as an interrupt, the irq_type was not available for > device driver. It is not align with devicetree probing. > > Signed-off-by: Christophe Ricard <christophe-h.ric...@st.com> Acked-by: Linus Walleij <linus.wall...@linaro.org> Rafael you can merge this into the ACPI tree. 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 v2] i2c: imx: make bus recovery through pinctrl optional
On Fri, Nov 20, 2015 at 10:45 PM, Li Yang <le...@freescale.com> wrote:
> - if (IS_ERR(i2c_imx->pinctrl)) {
> + /* bailout on -ENOMEM or -EPROBE_DEFER, continue for other errors */
> + if (PTR_ERR(i2c_imx->pinctrl) == -ENOMEM ||
> + PTR_ERR(i2c_imx->pinctrl) == -EPROBE_DEFER) {
This is one of the evils of deferred probe: you never know if
something will eventually turn up. It feels wrong to bail out
on deferred probe...
I have no better idea though.
Acked-by
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 v2 0/8] enable I2C devices behind I2C bus on Gen2
On Thu, Oct 1, 2015 at 1:20 PM, Andy Shevchenko <andriy.shevche...@linux.intel.com> wrote: > The patches 7 and 8 are pretty independent, though they don't make much sense > without previous ones applied. To me it seems patches 5 & 6 (GPIO patches) can be applied as-is to my GPIO tree without any bad side effects. Is this correct? In that case I will apply them. 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 v2 5/8] gpio: pca953x: store driver_data for future use
On Thu, Oct 1, 2015 at 1:20 PM, Andy Shevchenko <andriy.shevche...@linux.intel.com> wrote: > Instead of using id->driver_data directly we copied it to the internal > structure. This will help to adapt driver for ACPI use. > > Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com> Patch applied. 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 v2 6/8] gpio: pca953x: support ACPI devices found on Galileo Gen2
On Thu, Oct 1, 2015 at 1:20 PM, Andy Shevchenko <andriy.shevche...@linux.intel.com> wrote: > This patch adds a support of the expandes found on Intel Galileo Gen2 board. > The platform information comes from ACPI. > > Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com> Patch applied. 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 6/8] gpio: pca953x: support ACPI devices found on Galileo Gen2
On Tue, Sep 22, 2015 at 3:10 AM, Andy Shevchenko <andriy.shevche...@linux.intel.com> wrote: > This patch adds a support of the expandes found on Intel Galileo Gen2 board. > The platform information comes from ACPI. > > Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com> Paging Gregory, Grygorii, Graeme on this patch too. 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 5/8] gpio: pca953x: store driver_data for future use
On Tue, Sep 22, 2015 at 3:10 AM, Andy Shevchenko <andriy.shevche...@linux.intel.com> wrote: > Instead of using id->driver_data directly we copied it to the internal > structure. This will help to adapt driver for ACPI use. > > Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com> Gregory, Grygorii, Graeme: can I get some help in reviewing these PCA patches for ACPI support? 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 4/4] i2c: stu300: Fix module autoload for OF platform driver
On Thu, Sep 17, 2015 at 6:36 PM, Luis de Bethencourt <l...@debethencourt.com> wrote: > This platform driver has a OF device ID table but the OF module > alias information is not created so module autoloading won't work. > > Signed-off-by: Luis de Bethencourt <lui...@osg.samsung.com> Acked-by: Linus Walleij <linus.wall...@linaro.org> 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 v2] i2c: imx: implement bus recovery
On Mon, Sep 7, 2015 at 10:00 AM, Uwe Kleine-König <u.kleine-koe...@pengutronix.de> wrote: > [Me] >> If the use case is around the i2c traffic, it is a mode related to I2C, >> and if this mode is called "GPIO mode" in the data sheet >> is irrelevant, because it is obviously not used for the generic >> input/output but the specific I2C. The terminology should be >> made familiar to whoever needs to go in and read the code later. > > The background info that was obviously missing from the part of the > thread I sent to you is that pinctrl_pm_select_sleep_state is used to > prepare bitbanging on the i2c bus to do bus recovery. (The controller > doesn't implement this, so we have to resort to manually drive the > pins.) I don't understand. What does "manually" mean in this context? Code examples for "manual" and "automatic"? Generally the word "manual" is so ambigous that we should refrain from using it, to me that means something like hammering on a keyboard to alter things through sysfs or debugfs but I hardly believe this is what you mean. > Is this good enough? I'd like to see implemented a separate pinctrl set > for bitbanging instead of relying on the sleep state being the right > thing to enable gpio functionality. This sounds right but I need to see the two different code sets to understand. Now my head is all fuzzy... 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 v2] i2c: imx: implement bus recovery
On Wed, Aug 19, 2015 at 9:02 AM, Uwe Kleine-König
u.kleine-koe...@pengutronix.de wrote:
On Wed, Aug 19, 2015 at 03:44:49AM +, Gao Pandy wrote:
From: Uwe Kleine-König mailto:u.kleine-koe...@pengutronix.de Sent:
Thursday, August 13, 2015 4:15 PM
+static void i2c_imx_prepare_recovery(struct i2c_adapter *adap) {
+ struct imx_i2c_struct *i2c_imx;
+
+ i2c_imx = container_of(adap, struct imx_i2c_struct, adapter);
+ if (i2c_imx-pins.sda i2c_imx-pins.scl) {
+ pinctrl_pm_select_sleep_state(adap-dev);
Your requirement that the sleep state should configure the pins as gpio
is strange. Maybe better introduce a dedicated state for recovery? At
least you should document this requirement.
In general, pinctrl sleep mode is gpio function. I will add this in binding
doc. Thanks.
Linus, do you have to say something here? It might be right to have the
gpio function as configuration for sleep mode, but it doesn't look right
for me to use this for recovery purposes.
What it usually means is that the pin has a function mode such
that an asynchronous edge detector is connected to it on the
outer pad ring, maybe in tristate or some pull-up/down configuration.
This makes is possible for the system to power
off completely until an event occurs there with only some
very peripheral electronics powered up.
I think the terminology depend on the use case. See the section
GPIO mode pitfalls in Documentation/pinctrl.txt
If the use case is around the i2c traffic, it is a mode related to I2C,
and if this mode is called GPIO mode in the data sheet
is irrelevant, because it is obviously not used for the generic
input/output but the specific I2C. The terminology should be
made familiar to whoever needs to go in and read the code later.
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: [U-Boot] [PATCH v3 01/11] dm: serial: Update binding for PL01x serial UART
On Thu, Aug 13, 2015 at 9:04 PM, Ian Lepore i...@freebsd.org wrote: As the FreeBSD person who got our first SoC (imx6, only partially supported) converted to use the Linux DT files rather than our own homebrew mess we started with, I would say that my opinion of the existing DT information is that it is an extension of Linux device drivers written in a different language. This is the first time I hear a story like this, tell us more! This is exactly the kind of stuff we want to see posted on devicet...@vger.kernel.org. The information available is in no way independent of the Linux device drivers, it is exactly the information those drivers need. It is often not the information needed in another OS with independently written drivers. And especially it is not ordered and structured in a way that works well with the device enumeration and instantiation models used by another OS. I have complained before that since the bindings are often merged through the Linux kernel tree subsystem maintainers, they ultimately decide on bindings. Unsurprisingly, they are as unlikely to notice linuxisms as the Windows people designing ACPI are unlikely to notice Windowsisms. But atleast we know we are flawed and want to improve. The best way to improve is to have people from other OSes on the devicetree mailinglist and review the bindings there from other-OS point of view. A great case in point would be i2c eeproms. What a perfect opportunity DT would be to describe everything about the eeprom parts (total capacity, page read/write size, whether the page address bits extend into the bus-slave address bits, etc). It seems to me that anything claiming to be an independent description of the hardware would have to include such things. Instead, all the bindings define is the compatible string. That's crazy. Why? Well, when I went and looked at the Linux eeprom drivers it became clear why: that's all they need to know, because everything else is hard-coded in tables in the driver source. So if I want to write a FreeBSD i2c eeprom driver that uses DT data, what are my choices? I have exactly one: make my driver essentially a clone of the Linux driver, with all the same data hard-coded in source. I bet Wolfram and other I2C people can comment on this, state and future directions. Wolfram is known to care deeply about the problem with DT contracts. All in all, it's not impossible for another OS to work with the DT information that begins its life in Linux, but it's not really easy. Let's maker this better. 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 00/21] On-demand device registration
On Sat, Jun 13, 2015 at 8:27 PM, Alexander Holler hol...@ahsoftware.de wrote: And because you've said that problem space is a bit convoluted and I disagree, here's a summary from my point of view: 1. All the necessary information (dependencies between drivers) already exists at compile time. The set of dependencies between drivers might become smaller by configuration, but will not become larger. So there should be NO need to collect them at runtime, e.g. by instrumenting function calls. I think you arrived at the core of the crux here. When we look up a resource provided from another driver, e.g. from regulator_get(), clk_get(), pinctrl_get(), gpiod_get() etc - the dependency is resolved by looking in a cross-reference table for either a struct device* pointer or a string, an index, or both or all three. Examples: struct regulator *regulator_get(struct device *dev, const char *id); struct clk *clk_get(struct device *dev, const char *id); struct gpio_desc *__must_check __gpiod_get(struct device *dev, const char *con_id, enum gpiod_flags flags); (...) (*_index() variants exist on some of the resource retrieveal functions.) struct device * is the device requesting the resource, con_id is the string name of the resource on the provider side. This is all solved by looking in cross reference tables. ONE way of resolving that cross reference is to look into the device tree or the ACPI table. But for the board file case, this is resolved at runtime by the cross reference table, registered with calls such as gpiod_add_lookup_table(). It is true that in the theoretical sense, all of this exist at compile time especially if you can parse something like a device tree and figure out what struct device * nodes will correspond to the struct device_node:s in it. For ACPI I guess a similar procedure is viable. Problem: this requires the kernel compile to know exactly *which* device tree or ACPI table it is going to boot on. The expressed goal of device tree and ACPI is to have *ONE* kernel booting several device trees. Here your approach stops short: you are suggesting instrumenting the kernel at compile time to one single device tree or ACPI table. But we never know really what device tree or ACPI table will be used. This just cannot be done at compile time for that reason alone. Example: in boot case (A) the regulator may be provided by regulator foo driver on an i2c bus. But in boot case (B) the very same regulator may be provided by regulator bar on an SPI bus. These are very real usecases, for example for drivers/net/ethernet/smsc/smsc911x.c, will get regulators from the most diverse places depending on what device tree is used. For board files, it is neither possible in theory: you need to compile the code to figure out the struct device * provider, and/or the string name of the providing device (.name field in struct device for the provider) to resolve dependencies at compile time. For the board file case, resolving dependencies at compile time will require a quite complex two-stage rocket: compile the code to get resources out, then recompile with known resources. I guess your suggested approach then need to introduce a special build tool to order the initcalls accordingly. Again this will fall short if you don't know at compile time exactly *which* board file will be executed. So the only practical way to solve this at compile time is to predict an initcall ordering sequence for all possible boot paths, compile in all of them, and choose the right one at boot. But the number of boot paths is equal to the number of device trees / ACPI tables or board files supported, and that space is uncontrolled and ordered infinite. Basically I think the root problem with your approach is that you assume we know what hardware we will boot on at compile time. We discarded that development path years ago. We have no clue, this is resolved at runtime. Alas, people still create super-optimized systems using exactly this knowledge, but it is not our main target here, it is a special optimization case. 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 00/21] On-demand device registration
On Thu, Jun 11, 2015 at 6:40 PM, Alexander Holler hol...@ahsoftware.de wrote: Am 11.06.2015 um 14:30 schrieb Linus Walleij: Certainly it is possible to create deadlocks in this scenario, but the scope is not to create an ubreakable system. IAnd what happens if you run into a deadlock? Do you print you've lost, try changing your kernel config in some output hidden by a splash-screen? ;) Sorry it sounds like a blanket argument, the fact that there are mutexes in the kernel makes it possible to deadlock, it doesn't mean we don't use mutexes. Some programming problems are just like such. 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 00/21] On-demand device registration
On Thu, Jun 11, 2015 at 12:17 PM, Alexander Holler hol...@ahsoftware.de wrote: Am 11.06.2015 um 10:12 schrieb Linus Walleij: On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler hol...@ahsoftware.de wrote: You would end up with the same problem of deadlocks as currently, and you would still need something ugly like the defered probe brutforce to avoid them. Sorry I don't get that. Care to elaborate on why? Because loading/initializing on demand doesn't give you any solved order of drivers to initialize. And it can't because it has no idea about the requirements of other drivers. The reason why it might work better in the case of the tegra is that it might give you another initialization order than the one which is currently choosen, which, by luck, might be a better one. But maybe I missed something, I haven't looked at the patches at all. But just loading on demand, can't magically give you a working order of drivers to initialize. E.g. how do you choose the first driver to initialize? So the current patch set introduces dependencies (just for device tree) and Tomeu is working on a more generic dependency approach for any HW description. The first driver to initialize will be as usual the first one in the list for that initlevel, then walking up the initilevels. However if any driver runs into a resource roadblock it will postpone and wait for dependencies to probe first. Certainly it is possible to create deadlocks in this scenario, but the scope is not to create an ubreakable system. 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 00/21] On-demand device registration
On Wed, Jun 10, 2015 at 12:19 PM, Tomeu Vizoso tomeu.viz...@collabora.com wrote: On 10 June 2015 at 09:30, Linus Walleij linus.wall...@linaro.org wrote: regulator_get(...) - not available, so: - identify target regulator provider - this will need instrumentation - probe it It then turns out the regulator driver is on the i2c bus, so we need to probe the i2c driver: - identify target i2c host for the regulator driver - this will need instrumentation - probe the i2c host driver i2c host comes out, probes the regulator driver, regulator driver probes and then the regulator_get() call returns. Hmm, if I understand correctly what you say, this is exactly what this particular series does: regulator_get - of_platform_device_ensure - probe() on the platform device that encloses the requested device node (i2c host) - i2c slave gets probed and the regulator registered - regulator_get returns the requested resource Yes. But only for device tree. The downside I'm currently looking at is that an explicit dependency graph would be useful to have for other purposes. For example to print a neat warning when a dependency cannot be fulfilled. Or to refuse to unbind a device which other devices depend on, or to automatically unbind the devices that depend on it, or to print a warning if a device is hotplugged off and other devices depend on it. Unbind/remove() calls are the inverse usually yes. But also the [runtime] power up/down sequences for the devices tend to depend on a similar ordering or mostly the same. (Mentioned this before I think.) This requires instrumentation on anything providing a resource to another driver like those I mentioned and a lot of overhead infrastructure, but I think it's the right approach. However I don't know if I would ever be able to pull that off myself, I know talk is cheap and I should show the code instead. Yeah, if you can give it a second look and say if it matches what you wrote above, it would be very much appreciated. Yes you are right. But what about ACPI, board files, Simple Firmware and future hardware description languages... 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 00/21] On-demand device registration
On Tue, Jun 2, 2015 at 12:14 PM, Tomeu Vizoso tomeu.viz...@collabora.com wrote: On 2 June 2015 at 10:48, Linus Walleij linus.wall...@linaro.org wrote: This is what systemd is doing in userspace for starting services: ask for your dependencies and wait for them if they are not there. So drivers ask for resources and wait for them. It also needs to be abstract, so for example we need to be able to hang on regulator_get() until the driver is up and providing that regulator, and as long as everything is in slowpath it should be OK. (And vice versa mutatis mutandis for clk, gpio, pin control, interrupts (!) and DMA channels for example.) I understood above that you propose probing devices in order, but now you mention that resource getters would block until the dependency is fulfilled which confuses me because if we are probing in order then all dependencies would be fulfilled before the device in question gets probed. Sorry, the problem space is a bit convoluted so the answers get a bit convoluted. Maybe I'm thinking aloud and altering the course of my thoughts as I type... I guess there can be explicit dependencies for resources like this patch does, but another way would be for all resource fetch functions to be instrumented, so that you do not block until you try to take a resource that is not yet there, e.g.: regulator_get(...) - not available, so: - identify target regulator provider - this will need instrumentation - probe it It then turns out the regulator driver is on the i2c bus, so we need to probe the i2c driver: - identify target i2c host for the regulator driver - this will need instrumentation - probe the i2c host driver i2c host comes out, probes the regulator driver, regulator driver probes and then the regulator_get() call returns. This requires instrumentation on anything providing a resource to another driver like those I mentioned and a lot of overhead infrastructure, but I think it's the right approach. However I don't know if I would ever be able to pull that off myself, I know talk is cheap and I should show the code instead. Deepest respect for your efforts! 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 00/21] On-demand device registration
On Mon, May 25, 2015 at 4:53 PM, Tomeu Vizoso tomeu.viz...@collabora.com wrote: 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. This is pretty cool, but a too local solution to a global problem. Deferred probe and initcall reordering, silly as they may seem, does not require you to use device tree. The real solution, which I think I pointed out already when we added deferred probe, is to put dependency graphs in the drivers and have the kernel device driver core percolate dependecies by walking the graph on probing driver, removing driver (usually the inverse use case), [runtime] suspend and [runtime] resumeing a driver. Possibly the dependencies will even be different depending on use case. This is what systemd is doing in userspace for starting services: ask for your dependencies and wait for them if they are not there. So drivers ask for resources and wait for them. It also needs to be abstract, so for example we need to be able to hang on regulator_get() until the driver is up and providing that regulator, and as long as everything is in slowpath it should be OK. (And vice versa mutatis mutandis for clk, gpio, pin control, interrupts (!) and DMA channels for example.) So if this should be solved it should be solved in an abstract way in the device driver core available for all, then have calls calling out to DT, ACPI, possibly even PCI or USB (as these enumerate devices themselves) to obtain a certain dependency. 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 05/12] i2c: pxa: Add bus reset functionality
On Thu, May 28, 2015 at 3:03 PM, Vaibhav Hiremath
vaibhav.hirem...@linaro.org wrote:
From: Rob Herring r...@kernel.org
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
Double signed-off?
(...)
+#include linux/of_gpio.h
You should use linux/gpio/consumer.h and then use
GPIO descriptors instead.
@@ -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;
Yup that's the right way. I see this is the default
state for pin_i2c.
+static void i2c_bus_reset(struct pxa_i2c *i2c)
+{
+ int ret, ccnt, pins_scl, pins_sda;
Use GPIO descriptors.
struct gpio_desc *scl, *sda;
+ struct device *dev = i2c-adap.dev.parent;
+ struct device_node *np = dev-of_node;
+
+ if (!i2c-pinctrl) {
+ dev_warn(dev, could not do i2c bus reset\n);
+ return;
+ }
+
+ ret = pinctrl_select_state(i2c-pinctrl, i2c-pin_gpio);
+ if (ret) {
+ dev_err(dev, could not set gpio pins\n);
+ return;
+ }
Exactly like that yes.
+ pins_scl = of_get_named_gpio(np, i2c-gpio, 0);
+ if (!gpio_is_valid(pins_scl)) {
+ dev_err(dev, i2c scl gpio not set\n);
+ goto err_out;
+ }
+ pins_sda = of_get_named_gpio(np, i2c-gpio, 1);
+ if (!gpio_is_valid(pins_sda)) {
+ dev_err(dev, i2c sda gpio not set\n);
+ goto err_out;
+ }
I would suggest just using two GPIOs in the node relying
on index order. With GPIO descriptors:
scl = gpiod_get_index(dev, i2c-gpio, 0, GPIOD_ASIS);
sda = gpiod_get_index(dev, i2c-gpio, 1, GPIOD_ASIS);
Then use gpiod* accessors below and...
+
+ gpio_request(pins_scl, NULL);
+ gpio_request(pins_sda, NULL);
+
+ gpio_direction_input(pins_sda);
+ for (ccnt = 20; ccnt; ccnt--) {
+ gpio_direction_output(pins_scl, ccnt 0x01);
+ udelay(5);
+ }
+ gpio_direction_output(pins_scl, 0);
+ udelay(5);
+ gpio_direction_output(pins_sda, 0);
+ udelay(5);
+ /* stop signal */
+ gpio_direction_output(pins_scl, 1);
+ udelay(5);
+ gpio_direction_output(pins_sda, 1);
+ udelay(5);
+
+ gpio_free(pins_scl);
+ gpio_free(pins_sda);
gpiod_put(scl);
gpiod_put(sda);
+err_out:
+ ret = pinctrl_select_state(i2c-pinctrl, i2c-pin_i2c);
+ if (ret)
+ dev_err(dev, could not set default(i2c) pins\n);
+ return;
Nice.
Overall it looks like a real nice structured workaround using
the API exactly as intended, just need to catch up with
using GPIO descriptors.
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 1/2] i2c: nomadik: match return type of wait_for_completion_timeout
On Sun, Feb 8, 2015 at 1:34 PM, Nicholas Mc Guire hof...@osadl.org wrote: return type of wait_for_completion_timeout is unsigned long not int. as timeout is used for wait_for_completion_timeout exclusively here its type is simply changed to unsigned long. Signed-off-by: Nicholas Mc Guire hof...@osadl.org Acked-by: Linus Walleij linus.wall...@linaro.org 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 2/2] i2c: nomadik: match status to return type of read_i2c
On Sun, Feb 8, 2015 at 1:34 PM, Nicholas Mc Guire hof...@osadl.org wrote: return type of read_i2c() is int not u32. As the assignments to status are consistent with int here its type is changed to int. Signed-off-by: Nicholas Mc Guire hof...@osadl.org Acked-by: Linus Walleij linus.wall...@linaro.org 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 v4 3/3] gpio: add support for Cypress CYUSBS234 USB-GPIO adapter
of the packet format to reference in that comment? 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: [GIT PULL] Immutable branch between MFD, GPIO and I2C
On Mon, Nov 10, 2014 at 6:43 PM, Lee Jones lee.jo...@linaro.org wrote: Please don't pull this -- it is missing a patch. Will fix. Okay, dependency fixed. Sorry for the fuss. Pull when ready. Letting it just sit around unless there are conflicts coming up... Seems like this can go through MFD alone from the GPIO side of things. 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 v3 3/3] gpio: add support for Cypress CYUSBS234 USB-GPIO adapter
On Thu, Oct 9, 2014 at 1:46 AM, RR rajaram.officema...@gmail.com wrote:
On Wed, Oct 8, 2014 at 12:18 AM, Alexandre Courbot gnu...@gmail.com wrote:
On Wed, Oct 8, 2014 at 4:09 PM, Muthu Mani m...@cypress.com wrote:
+static int cy_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value) {
+ return 0;
+}
If that chip is capable of both output and input, shouldn't these
functions be
implemented? I think this has already been pointed out in a previous
version
but you did not reply.
Thanks for your inputs.
Only the GPIOs which are configured to be output GPIO can be set.
In that case cy_gpio_set() should return an error for GPIOs which are
not configured as outputs. Is that guaranteed by the current
implementation?
The set operation would fail trying to set the input or unconfigured GPIOs.
In this version of driver, this support is not added, it can be introduced
in future versions.
I will add a TODO note in the code.
Argh, no TODO please. Actual code that will turn this code into a
solid driver that can be merged.
Does a driver targeted for a custom device has to implement every
functionality in the 1st version ?
When you post a driver to the GPIO maintainers it is *NOT* tageted
at a consumer device, it is targeted at the kernel community and
upstream maintainers.
Of course you can deliver add-on patches out-of-tree to your
customers, it's generally a bad idea for the long term and maintenance
of your driver, but it's your pick.
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 v7 3/4] gpiolib: add irq_not_threaded flag to gpio_chip
On Thu, Oct 9, 2014 at 9:22 PM, Octavian Purdila octavian.purd...@intel.com wrote: Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set operation but do not need a threaded irq handler. Signed-off-by: Octavian Purdila octavian.purd...@intel.com This is already upstream now. Rebase and you can drop this patch. 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 v7 4/4] gpio: add support for the Diolan DLN-2 USB GPIO driver
On Thu, Oct 9, 2014 at 9:22 PM, Octavian Purdila octavian.purd...@intel.com wrote: From: Daniel Baluta daniel.bal...@intel.com This patch adds GPIO and IRQ support for the Diolan DLN-2 GPIO module. Information about the USB protocol interface can be found in the Programmer's Reference Manual [1], see section 2.9 for the GPIO module commands and responses. [1] https://www.diolan.com/downloads/dln-api-manual.pdf Signed-off-by: Daniel Baluta daniel.bal...@intel.com Signed-off-by: Octavian Purdila octavian.purd...@intel.com Looks good to me. Acked-by: Linus Walleij linus.wall...@linaro.org 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 v5 3/4] gpiolib: add irq_not_threaded flag to gpio_chip
On Fri, Sep 19, 2014 at 10:22 PM, Octavian Purdila octavian.purd...@intel.com wrote: Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set operation but do not need a threaded irq handler. Signed-off-by: Octavian Purdila octavian.purd...@intel.com Usually I don't apply patches adding interfaces with no users, but this seems very useful, so patch applied. I guess your driver will appear on v3.19+ so then you can rely on this having been merged for v3.18. 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 3/3] gpio: add support for the Diolan DLN-2 USB-GPIO driver
On Wed, Aug 20, 2014 at 1:24 PM, Daniel Baluta daniel.bal...@intel.com wrote: This patch adds GPIO and IRQ support for the Diolan DLN-2 GPIO module. Information about the USB protocol interface can be found in the Programmer's Reference Manual [1], see section 2.9 for the GPIO module commands and responses. [1] https://www.diolan.com/downloads/dln-api-manual.pdf Signed-off-by: Daniel Baluta daniel.bal...@intel.com Major change required: rewrite this driver to not select IRQ_DOMAIN, instead select GPIOLIB_IRQCHIP and use the shared infrastructure. See other drivers that select GPIOLIB_IRQCHIP or grep the git log to see how this works in practice. You need to use some container_of() operations. 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 14/16] i2c: i2c-stu300: Drop class based scanning to improve bootup time
On Thu, Jul 10, 2014 at 1:46 PM, Wolfram Sang w...@the-dreams.de wrote: This driver has been flagged to drop class based instantiation. The removal improves boot-up time and is unneeded for embedded controllers. Users have been warned to switch for some time now, so we can actually do the removal. Keep the DEPRECATED flag, so the core can inform users that the behaviour finally changed now. After another transition period, this flag can go, too. Signed-off-by: Wolfram Sang w...@the-dreams.de Acked-by: Linus Walleij linus.wall...@linaro.org 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 08/16] i2c: i2c-nomadik: Drop class based scanning to improve bootup time
On Thu, Jul 10, 2014 at 1:46 PM, Wolfram Sang w...@the-dreams.de wrote: This driver has been flagged to drop class based instantiation. The removal improves boot-up time and is unneeded for embedded controllers. Users have been warned to switch for some time now, so we can actually do the removal. Keep the DEPRECATED flag, so the core can inform users that the behaviour finally changed now. After another transition period, this flag can go, too. While we are here, remove the indentation for the array setup because such things always break after some time. Signed-off-by: Wolfram Sang w...@the-dreams.de Acked-by: Linus Walleij linus.wall...@linaro.org 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] i2c: Make I2C ID tables non-mandatory for DT'ed and/or ACPI'ed devices
On Wed, Jun 4, 2014 at 8:09 AM, Michael Lawnick ml.lawn...@gmx.de wrote: Am 03.06.2014 13:18, schrieb Linus Walleij: On Mon, Jun 2, 2014 at 4:29 PM, Michael Lawnick ml.lawn...@gmx.de wrote: Am 02.06.2014 14:16, schrieb Linus Walleij: Is this really so useful on embedded systems? I was under the impression that this method was something used on say PC desktops with temperature monitors and EEPROMs on some I2C link on the PCB, usage entirely optional and fun for userspace hacks. We use it for dynamic instantiating whole subsystems with multiplexers, sensors, controllers in an embedded system. The device list is taken from an I2C eeprom which gets read on hotplug. Does this mean that you have stored the names (strings) that are used by the Linux kernel for identifying the devices into your EEPROM? That means that you have made the kernel-internal device driver names an ABI which is unfortunate :-/ This is one of the reasons to why we insist on device tree: OS neutral hardware description. The eeprom contains a device tree that is dynamically merged. That is a kind of way of saying yes we made the kernel-internal driver named an ABI I guess? 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
[PATCH] i2c: mux: pca954x: fix dependencies
This driver causes the following randconfig build error: drivers/i2c/muxes/i2c-mux-pca954x.c: In function ‘pca954x_probe’: drivers/i2c/muxes/i2c-mux-pca954x.c:204:2: error: implicit declaration of function ‘devm_gpiod_get’ [-Werror=implicit-function-declaration] gpio = devm_gpiod_get(client-dev, reset); ^ drivers/i2c/muxes/i2c-mux-pca954x.c:204:7: warning: assignment makes pointer from integer without a cast [enabled by default] gpio = devm_gpiod_get(client-dev, reset); ^ drivers/i2c/muxes/i2c-mux-pca954x.c:206:3: error: implicit declaration of function ‘gpiod_direction_output’ [-Werror=implicit-function-declaration] gpiod_direction_output(gpio, 0); ^ cc1: some warnings being treated as errors make[3]: *** [drivers/i2c/muxes/i2c-mux-pca954x.o] Error 1 This is because it is getting compiled without gpiolib, so introduce an explicit dependency. Reported-by: Jim Davis jim.ep...@gmail.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/i2c/muxes/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig index f7f9865b8b89..f6d313e528de 100644 --- a/drivers/i2c/muxes/Kconfig +++ b/drivers/i2c/muxes/Kconfig @@ -40,6 +40,7 @@ config I2C_MUX_PCA9541 config I2C_MUX_PCA954x tristate Philips PCA954x I2C Mux/switches + depends on GPIOLIB help If you say yes here you get support for the Philips PCA954x I2C mux/switch devices. -- 1.9.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: randconfig build error with next-20140604, in drivers/i2c/muxes/i2c-mux-pca954x.c
On Wed, Jun 4, 2014 at 6:44 PM, Jim Davis jim.ep...@gmail.com wrote: Building with the attached random configuration file, drivers/i2c/muxes/i2c-mux-pca954x.c: In function ‘pca954x_probe’: drivers/i2c/muxes/i2c-mux-pca954x.c:204:2: error: implicit declaration of function ‘devm_gpiod_get’ [-Werror=implicit-function-declaration] gpio = devm_gpiod_get(client-dev, reset); ^ drivers/i2c/muxes/i2c-mux-pca954x.c:204:7: warning: assignment makes pointer from integer without a cast [enabled by default] gpio = devm_gpiod_get(client-dev, reset); ^ drivers/i2c/muxes/i2c-mux-pca954x.c:206:3: error: implicit declaration of function ‘gpiod_direction_output’ [-Werror=implicit-function-declaration] gpiod_direction_output(gpio, 0); ^ cc1: some warnings being treated as errors make[3]: *** [drivers/i2c/muxes/i2c-mux-pca954x.o] Error 1 I've sent a patch for this to Wolfram and the i2c discuss list. 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] i2c: Make I2C ID tables non-mandatory for DT'ed and/or ACPI'ed devices
On Sat, May 31, 2014 at 3:48 PM, Wolfram Sang w...@the-dreams.de wrote: Right, I read the function which provides the functionality, but my point is; I don't think my patch changes the semantics in a way which would adversely affect this option. If you think that it does, can you specify how please? Currently, if a driver would be DT only and does not provide a seperate i2c_device_id table, then the driver is unusable with method 4. I don't like to have some drivers being capable of it and some not. Does the sysfs method create a i2c_device_id table? If not, how does it probe successfully pre-patch? The sysfs method creates a device. Its name is matched against i2c_device_ids only since it does not have a node pointer for DT to be matched against. Is this really so useful on embedded systems? I was under the impression that this method was something used on say PC desktops with temperature monitors and EEPROMs on some I2C link on the PCB, usage entirely optional and fun for userspace hacks. And when we say people use it do we mean sensors-detect uses it, on desktops, really? 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] i2c: Make I2C ID tables non-mandatory for DT'ed and/or ACPI'ed devices
On Mon, Jun 2, 2014 at 2:38 PM, Wolfram Sang w...@the-dreams.de wrote: Though, I wouldn't mind if compatible entries could be passed to the 'new_device' file, in addition to i2c_device_ids. Yet, this needs some extra handling I haven't found the time for, yet. Hm that's a way forward then I guess... but passing a compatible string to that same file is a bit arbitrarily ambigous. So we'd rather add a new instatiation file named new_device_of_compatible or so? 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 13/16] i2c: stu300: remove unnecessary OOM messages
On Wed, May 7, 2014 at 6:29 AM, Jingoo Han jg1@samsung.com wrote: The site-specific OOM messages are unnecessary, because they duplicate the MM subsystem generic OOM message. Signed-off-by: Jingoo Han jg1@samsung.com Acked-by: Linus Walleij linus.wall...@linaro.org 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] i2c: nomadik: Don't use IS_ERR for devm_ioremap
On Tue, May 6, 2014 at 11:32 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 10 April 2014 16:19, Ulf Hansson ulf.hans...@linaro.org wrote: devm_ioremap() returns NULL on error, not an error. Cc: Alessandro Rubini rub...@unipv.it Cc: Linus Walleij linus.wall...@linaro.org Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Linus, Wolfram - ping. Acked-by: Linus Walleij linus.wall...@linaro.org 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] i2c: nomadik: Fixup system suspend
On Thu, Apr 10, 2014 at 3:59 PM, Ulf Hansson ulf.hans...@linaro.org wrote: For !CONFIG_PM_RUNTIME, the device were never put back into active state while resuming. For CONFIG_PM_RUNTIME, we blindly trusted the device to be inactive while we were about to handle it at suspend late, which is just too optimistic. Even if the driver uses pm_runtime_put_sync() after each tranfer to return it's runtime PM resources, there are no guarantees this will actually mean the device will inactivated. The reason is that the PM core will prevent runtime suspend during system suspend, and thus when a transfer occurs during the early phases of system suspend the device will be kept active after the transfer. To handle both issues above, use pm_runtime_force_suspend|resume() from the system suspend|resume callbacks. Cc: Alessandro Rubini rub...@unipv.it Cc: Linus Walleij linus.wall...@linaro.org Cc: Wolfram Sang w...@the-dreams.de Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org 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 v2] i2c: nomadik: factor platform data into state container
On Mon, Feb 24, 2014 at 10:13 AM, Wolfram Sang w...@the-dreams.de wrote: On Mon, Feb 24, 2014 at 09:57:05AM +0100, Linus Walleij wrote: I can easily fix that up ipso facto by modifying the device trees to state 400kHz for them. Then lets do it this way. After a check I see that this is already in place in the device trees. 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 03/17] mmc: mmci: Mask IRQs for all variants during runtime suspend
On Tue, Feb 18, 2014 at 5:36 PM, Ulf Hansson ulf.hans...@linaro.org wrote: On 18 February 2014 17:05, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Feb 04, 2014 at 04:58:44PM +0100, Ulf Hansson wrote: In runtime suspended state, we are not expecting IRQs and thus we can safely mask them, not only for pwrreg_nopower variants but for all. Obviously we then also need to make sure we restore the IRQ mask while becoming runtime resumed. So, what happens when this patch is applied, and a SDIO card is attached which expects to receive interrupts at any moment? Currently, no variant implements SDIO irq. The SDIO irq polling mode from the sdio core will still be functional, as of today. So, this patch will not break SDIO. Given that I've run into this during the last week with a SDHCI controller, I'm not that thrilled with other interfaces doing the same broken thing. If we add SDIO irq support to mmci in future; parts of that implementation includes a re-route of DAT1 to a GPIO irq when entering runtime suspend state. The mmci HW will in runtime suspend state, not be responsible for handling irqs, which is the same as of today. [Just smalltalk] Switching DAT1 to gpio mode (which is something of a fallacy, see explanation in Documentation/pinctrl.txt) is not at all possible in all implementations of the PL18x, as it depends on exploiting properties on an assumed pin controller. Systems that don't have such wakeup features on their pins are unlikely to support deepsleep in any capacity, and if they do they are ill-designed from the top level as this needs to be taken into account when devising the hardware :-/ 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 v2] i2c: nomadik: factor platform data into state container
On Mon, Feb 3, 2014 at 11:27 AM, Linus Walleij linus.wall...@linaro.org wrote: Move the former platform data struct nmk_i2c_controller into the per-device state container struct i2c_nmk_client, and remove all the platform data probe path hacks. Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org --- ChangeLog v1-v2: - Drop pointless check for np, as the device can only probe from the device tree now. Wolfram, ping on this! 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 15/17] i2c: i2c-stu300: deprecate class based instantiation
On Mon, Feb 10, 2014 at 11:04 AM, Wolfram Sang w...@the-dreams.de wrote: Warn users that class based instantiation is going away soon in favour of more robust probing and faster bootup times. Signed-off-by: Wolfram Sang w...@the-dreams.de Cc: Linus Walleij linus.wall...@linaro.org --- This patch is a suggestion. Looking for an ack by someone who actually uses the driver. Acked-by: Linus Walleij linus.wall...@linaro.org 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 10/17] i2c: i2c-nomadik: deprecate class based instantiation
On Mon, Feb 10, 2014 at 11:04 AM, Wolfram Sang w...@the-dreams.de wrote: Warn users that class based instantiation is going away soon in favour of more robust probing and faster bootup times. Signed-off-by: Wolfram Sang w...@the-dreams.de Cc: Alessandro Rubini rub...@unipv.it Cc: Linus Walleij linus.wall...@linaro.org --- This patch is a suggestion. Looking for an ack by someone who actually uses the driver. Acked-by: Linus Walleij linus.wall...@linaro.org 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 10/17] spi: pl022: Remove redundant pinctrl to default state in probe
On Tue, Feb 4, 2014 at 4:58 PM, Ulf Hansson ulf.hans...@linaro.org wrote: The driver core is now taking care of putting our pins into default state at probe. Thus we can remove the redundant call for it in probe. Cc: Mark Brown broo...@kernel.org Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org 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 11/17] i2c: nomadik: Convert to devm functions
On Tue, Feb 4, 2014 at 4:58 PM, Ulf Hansson ulf.hans...@linaro.org wrote: Use devm_* functions to simplify code and error handling. Cc: Alessandro Rubini rub...@unipv.it Cc: Linus Walleij linus.wall...@linaro.org Cc: Wolfram Sang w...@the-dreams.de Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org However make sure this (and the rest) applies on top of this patch: http://marc.info/?l=linux-i2cm=139142325809973w=2 Because I expect that to be applied first. 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 14/17] i2c: nomadik: Fixup deployment of runtime PM
On Tue, Feb 4, 2014 at 4:58 PM, Ulf Hansson ulf.hans...@linaro.org wrote: Since the device is active while a successful probe has been completed, the reference counting for the clock will be screwed up and never reach zero. The issue is resolved by implementing runtime PM callbacks and let them handle the resources accordingly, including the clock. Cc: Alessandro Rubini rub...@unipv.it Cc: Linus Walleij linus.wall...@linaro.org Cc: Wolfram Sang w...@the-dreams.de Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Hm do I read it right as patch 13 breaks runtime PM by leaving the device active after probe() and this patch 14 fixes it again? Maybe these two patches should be squashed then. 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: More GPIO madness on iMX6 - and the crappy ARM port of Linux
On Tue, Jan 21, 2014 at 3:55 AM, Alexandre Courbot gnu...@gmail.com wrote: On Sat, Jan 18, 2014 at 7:43 AM, Linus Walleij linus.wall...@linaro.org wrote: 1. Today this OPEN_DRAIN flag is not even passed down to the driver so how could it say anything about it :-( it's a pure gpiolib internal flag. We don't know if the hardware can actually even do open drain, we just assume it can. What it should really do - in the best of worlds - is to check if it can cross-reference the GPIO line to a pin in the pin control subsystem, and if that is possible, then ask the pin if it is supporting open drain and set it. It currently has no such cross-calls, it is just assumed that the configuration is consistent, and the actual pin is set up as open drain. But it would make sense to add more cross-calls here, since GPIO is accepting these flags (OPEN_DRAIN/OPEN_SOURCE). This would definitely work in the case of pinctrl-backed GPIOs, but would not cover all GPIO chips. If we want to cover all cases we should give drivers a way to way to report or enforce this capability, and make the pinctrl cross-reference one of its implementations where it can be done. It can never be done in all cases, since in some cases the open drain config is just a property of the line and not controlled by software at all, and the datasheet just says this line is open drain. But we should cover the cases where we deal with pure SW-controlled configs as far as possible. Alexandre: do you have plans for how to handle a dynamic consumer passing flags to its gpio request in the gpiod API? Do you mean like passing OPEN_DRAIN or OPEN_SOURCE flags to gpiod_get(), similarly to what is done for e.g. gpio_request_one()? Yes... In the case of the gpiod API I would rather see these flags defined in the GPIO mapping if possible. For platform data it is already possible to specify open drain/open source, for DT this is trivial to add. I figured so much. But we have a consumer in i2c-core.c doing this for a valid reason. ACPI would be more of a problem here, but I'm not sure whether the problem is relevant for ACPI GPIOs. ACPI has an open drain/source flag for some GPIO lines IIRC. 1) GPIO drivers' request() function get an extra flags argument that is passed by the GPIO core with the flags of the mapping. There we can define all the range of properties that gpio_request_one() supported. The driver's request() will fail it if cannot satisfy these properties. That's where the pinctrl cross-reference would take place. I think this doesn't need to go all the way down into the driver actually. We can call to pinctrl in the gpiolib core and keep the gpiochip API simple. The GPIO driver doesn't even need to know this. 2) All properties accepted by gpio_request_one() can also be passed through GPIO mappings. That is, probably platform_data and DT. I don't know if we ever get to use open drain GPIOs provided by ACPI, if we do then this might be a problem. I think we need that. Like: bool gpiod_input_always_valid(const struct gpio_desc *desc); And leave it up to the core to look at flags, driver characteristics etc and determine whether the input can be trusted? I am personally a little bit scared by the number of exported functions in the GPIO framework. It is a pretty large number for something that is supposed to be simple, so I'd like to avoid adding more. :) I objected already when the OPEN_DRAIN et al were added, but to no avail... How about the following: 1) GPIOs configured as output without the open drain or open source flag either return -EINVAL on gpiod_get_value(), or a cached value tracked by gpiolib for consistency (probably the latter). Make sense. 2) GPIOs configured as open drain or open source report the actual value read on the pin, like i2c-core needs. This requires that, for each GPIO that can be set open drain or open source, gpiod_input_always_valid() == true. Yeah, but as seen from the I2C core, the algorithm there needs to know if the input is always valid or not, and take different execution paths depending on that. :-/ 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: More GPIO madness on iMX6 - and the crappy ARM port of Linux
On Fri, Jan 17, 2014 at 9:53 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Fri, Jan 17, 2014 at 01:42:44PM -0700, Stephen Warren wrote: I believe you want gpio_get_value() to return either the driven or actual pin value where it can on the current HW, but just e.g. hard-code 0 on other HW. That would introduce a core feature that works some places but not others, and hence make drivers that relied on the feature less portable between HW with different actual features. I can buy that argument, but there's an issue which stands squarely in its way, and that is open-drain GPIOs. These are modelled just as any other GPIO, mainly so that both gpio_set_value(gpio, 1) and gpio_direction_input(gpio) both result in the signal being high. The only combination which results in the signal being driven low is outputting zero - and the state of the signal can aways be read back. The problem here is that such gpios are implemented in things like the I2C driver such that they're _always_ outputs, and gpio_set_value() is used to pull the signal down. gpio_get_value() is used to read its current state. So, if we say that gpio_get_value() is undefined, we force such subsystems to always jump through the non-open-drain paths (using gpio_direction_input() to set the line high and gpio_direction_output(gpio, 0) to drive it low.) Incidentally that is what gpiolib is doing internally in gpiod_direction_output(). You're absolutely right that it makes no sense to have open drain (or open source) unless the signal can be read back from the hardware. I'm thinking something like if the driver manages to obtain a GPIO with gpio_request_one(gpio, GPIOF_OPEN_DRAIN | GPIOF_OUT_INIT_HIGH); As the I2C core does, and then when that call succeeds, it can expect that whatever comes back from gpio_get_value() is always what is actually on the line. If the driver cannot determine this it should not have allowed that flag to succeed in the first place, so this might be something we want to enforce. There are two white spots on the map here: 1. Today this OPEN_DRAIN flag is not even passed down to the driver so how could it say anything about it :-( it's a pure gpiolib internal flag. We don't know if the hardware can actually even do open drain, we just assume it can. What it should really do - in the best of worlds - is to check if it can cross-reference the GPIO line to a pin in the pin control subsystem, and if that is possible, then ask the pin if it is supporting open drain and set it. It currently has no such cross-calls, it is just assumed that the configuration is consistent, and the actual pin is set up as open drain. But it would make sense to add more cross-calls here, since GPIO is accepting these flags (OPEN_DRAIN/OPEN_SOURCE). Like: int pinctrl_gpio_set_flags(unsigned gpio, unsigned long flags); Where the pinctrl subsystem would attempt to cross reference and set the flag, and the pin controller backend will then have the option to return an error code. We could atleast support that for the select pin controllers that use generic pin config. i.MX is another story, but I'm open to compromises. 2. In the new descriptor API this open drain setting would be set from the lookup table and be a property on the line, meaning this flag is not requested explicitly by the consumer, and the consumer needs to inspect the obtained descriptor to figure out if it is set to open drain. Alexandre: do you have plans for how to handle a dynamic consumer passing flags to its gpio request in the gpiod API? I noticed that API missing now, there is exactly one user in the entire kernel, in drivers/i2c/i2c-core.c but a very important one. I guess to switch the I2C core over to descriptors I could think of an API like this: int gpiod_get_flags(const struct gpio_desc *desc); If the OPEN_DRAIN flag is set on that descriptor we should always be able to read the input. But as this is not really what the I2C core wants to know (it really would prefer not to bother with such GPIO flag details) so is it better if we add a special call to figure out if the input can be read? Like: bool gpiod_input_always_valid(const struct gpio_desc *desc); And leave it up to the core to look at flags, driver characteristics etc and determine whether the input can be trusted? 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 3/3] i2c: nomadik: factor platform data into state container
On Fri, Jan 3, 2014 at 4:35 PM, Wolfram Sang w...@the-dreams.de wrote:
+ if (!np) {
+ dev_err(adev-dev, no device node\n);
+ return -ENODEV;
Can this really happen? How should driver and device match otherwise?
Nah. I'll fix.
Rest is looking good.
Does this mean that you applied patch 1/3 and 2/3?
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 2/3] i2c: nomadik: remove platform data header
On Sat, Dec 14, 2013 at 1:19 AM, Linus Walleij linus.wall...@linaro.org wrote: On Thu, Nov 28, 2013 at 11:12 PM, Linus Walleij linus.wall...@linaro.org wrote: The Nomadik I2C is now configured from the device tree on all platforms using this controller. Delete the platform data header and move the definitions into the driver so it is all contained in one single file. Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org Wolfram: ping on this. Ping on this again. 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 3/3] i2c: nomadik: factor platform data into state container
On Sat, Dec 14, 2013 at 1:20 AM, Linus Walleij linus.wall...@linaro.org wrote: On Thu, Nov 28, 2013 at 11:12 PM, Linus Walleij linus.wall...@linaro.org wrote: Move the former platform data struct nmk_i2c_controller into the per-device state container struct i2c_nmk_client, and remove all the platform data probe path hacks. Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org Wolfram: ping on this. Ping on this again. 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 1/3] i2c: nomadik: auto-calculate slave setup time
On Thu, Nov 28, 2013 at 11:11 PM, Linus Walleij linus.wall...@linaro.org wrote: The Nomadik I2C controller needs to have the slave set-up time configured based off the clock used to drive the I2C bus block. Currently this is done with static assignments assuming that the block is clocked 48MHz which is pretty likely to be bug-prone. Calculate the SLSU from the equation given in the datasheet instead. Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org Wolfram: ping on this. 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 2/3] i2c: nomadik: remove platform data header
On Thu, Nov 28, 2013 at 11:12 PM, Linus Walleij linus.wall...@linaro.org wrote: The Nomadik I2C is now configured from the device tree on all platforms using this controller. Delete the platform data header and move the definitions into the driver so it is all contained in one single file. Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org Wolfram: ping on this. 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 3/3] i2c: nomadik: factor platform data into state container
On Thu, Nov 28, 2013 at 11:12 PM, Linus Walleij linus.wall...@linaro.org wrote: Move the former platform data struct nmk_i2c_controller into the per-device state container struct i2c_nmk_client, and remove all the platform data probe path hacks. Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org Wolfram: ping on this. 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
[PATCH 2/3] i2c: nomadik: remove platform data header
The Nomadik I2C is now configured from the device tree on all platforms
using this controller. Delete the platform data header and move the
definitions into the driver so it is all contained in one single file.
Cc: Lee Jones lee.jo...@linaro.org
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/i2c/busses/i2c-nomadik.c | 24 +-
include/linux/platform_data/i2c-nomadik.h | 34 ---
2 files changed, 23 insertions(+), 35 deletions(-)
delete mode 100644 include/linux/platform_data/i2c-nomadik.h
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 51e61d8127cb..4443613514ee 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -22,7 +22,6 @@
#include linux/clk.h
#include linux/io.h
#include linux/pm_runtime.h
-#include linux/platform_data/i2c-nomadik.h
#include linux/of.h
#include linux/pinctrl/consumer.h
@@ -104,6 +103,29 @@
/* maximum threshold value */
#define MAX_I2C_FIFO_THRESHOLD 15
+enum i2c_freq_mode {
+ I2C_FREQ_MODE_STANDARD, /* up to 100 Kb/s */
+ I2C_FREQ_MODE_FAST, /* up to 400 Kb/s */
+ I2C_FREQ_MODE_HIGH_SPEED, /* up to 3.4 Mb/s */
+ I2C_FREQ_MODE_FAST_PLUS,/* up to 1 Mb/s */
+};
+
+/**
+ * struct nmk_i2c_controller - client specific controller configuration
+ * @clk_freq: clock frequency for the operation mode
+ * @tft: Tx FIFO Threshold in bytes
+ * @rft: Rx FIFO Threshold in bytes
+ * @timeoutSlave response timeout(ms)
+ * @sm:speed mode
+ */
+struct nmk_i2c_controller {
+ u32 clk_freq;
+ unsigned char tft;
+ unsigned char rft;
+ int timeout;
+ enum i2c_freq_mode sm;
+};
+
/**
* struct i2c_vendor_data - per-vendor variations
* @has_mtdws: variant has the MTDWS bit
diff --git a/include/linux/platform_data/i2c-nomadik.h
b/include/linux/platform_data/i2c-nomadik.h
deleted file mode 100644
index 8681893f7b66..
--- a/include/linux/platform_data/i2c-nomadik.h
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
- * Copyright (C) 2009 ST-Ericsson
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2, as
- * published by the Free Software Foundation.
- */
-#ifndef __PDATA_I2C_NOMADIK_H
-#define __PDATA_I2C_NOMADIK_H
-
-enum i2c_freq_mode {
- I2C_FREQ_MODE_STANDARD, /* up to 100 Kb/s */
- I2C_FREQ_MODE_FAST, /* up to 400 Kb/s */
- I2C_FREQ_MODE_HIGH_SPEED, /* up to 3.4 Mb/s */
- I2C_FREQ_MODE_FAST_PLUS,/* up to 1 Mb/s */
-};
-
-/**
- * struct nmk_i2c_controller - client specific controller configuration
- * @clk_freq: clock frequency for the operation mode
- * @tft: Tx FIFO Threshold in bytes
- * @rft: Rx FIFO Threshold in bytes
- * @timeoutSlave response timeout(ms)
- * @sm:speed mode
- */
-struct nmk_i2c_controller {
- u32 clk_freq;
- unsigned char tft;
- unsigned char rft;
- int timeout;
- enum i2c_freq_mode sm;
-};
-
-#endif /* __PDATA_I2C_NOMADIK_H */
--
1.8.3.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 3/3] i2c: nomadik: factor platform data into state container
Move the former platform data struct nmk_i2c_controller into the
per-device state container struct i2c_nmk_client, and remove all
the platform data probe path hacks.
Cc: Lee Jones lee.jo...@linaro.org
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/i2c/busses/i2c-nomadik.c | 116 +++
1 file changed, 45 insertions(+), 71 deletions(-)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 4443613514ee..bc8ba02de2e9 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -111,22 +111,6 @@ enum i2c_freq_mode {
};
/**
- * struct nmk_i2c_controller - client specific controller configuration
- * @clk_freq: clock frequency for the operation mode
- * @tft: Tx FIFO Threshold in bytes
- * @rft: Rx FIFO Threshold in bytes
- * @timeoutSlave response timeout(ms)
- * @sm:speed mode
- */
-struct nmk_i2c_controller {
- u32 clk_freq;
- unsigned char tft;
- unsigned char rft;
- int timeout;
- enum i2c_freq_mode sm;
-};
-
-/**
* struct i2c_vendor_data - per-vendor variations
* @has_mtdws: variant has the MTDWS bit
* @fifodepth: variant FIFO depth
@@ -174,8 +158,12 @@ struct i2c_nmk_client {
* @irq: interrupt line for the controller.
* @virtbase: virtual io memory area.
* @clk: hardware i2c block clock.
- * @cfg: machine provided controller configuration.
* @cli: holder of client specific data.
+ * @clk_freq: clock frequency for the operation mode
+ * @tft: Tx FIFO Threshold in bytes
+ * @rft: Rx FIFO Threshold in bytes
+ * @timeout Slave response timeout (ms)
+ * @sm: speed mode
* @stop: stop condition.
* @xfer_complete: acknowledge completion for a I2C message.
* @result: controller propogated result.
@@ -188,8 +176,12 @@ struct nmk_i2c_dev {
int irq;
void __iomem*virtbase;
struct clk *clk;
- struct nmk_i2c_controller cfg;
struct i2c_nmk_client cli;
+ u32 clk_freq;
+ unsigned char tft;
+ unsigned char rft;
+ int timeout;
+ enum i2c_freq_mode sm;
int stop;
struct completion xfer_complete;
int result;
@@ -386,7 +378,7 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
* slsu = cycles / (10 / f) + 1
*/
ns = DIV_ROUND_UP_ULL(10ULL, i2c_clk);
- switch (dev-cfg.sm) {
+ switch (dev-sm) {
case I2C_FREQ_MODE_FAST:
case I2C_FREQ_MODE_FAST_PLUS:
slsu = DIV_ROUND_UP(100, ns); /* Fast */
@@ -409,7 +401,7 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
* 2 whereas it is 3 for fast and fastplus mode of
* operation. TODO - high speed support.
*/
- div = (dev-cfg.clk_freq 10) ? 3 : 2;
+ div = (dev-clk_freq 10) ? 3 : 2;
/*
* generate the mask for baud rate counters. The controller
@@ -419,7 +411,7 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
* so set brcr1 to 0.
*/
brcr1 = 0 16;
- brcr2 = (i2c_clk/(dev-cfg.clk_freq * div)) 0x;
+ brcr2 = (i2c_clk/(dev-clk_freq * div)) 0x;
/* set the baud rate counter register */
writel((brcr1 | brcr2), dev-virtbase + I2C_BRCR);
@@ -430,7 +422,7 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
* TODO - support for fast mode plus (up to 1Mb/s)
* and high speed (up to 3.4 Mb/s)
*/
- if (dev-cfg.sm I2C_FREQ_MODE_FAST) {
+ if (dev-sm I2C_FREQ_MODE_FAST) {
dev_err(dev-adev-dev,
do not support this mode defaulting to std. mode\n);
brcr2 = i2c_clk/(10 * 2) 0x;
@@ -438,11 +430,11 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
writel(I2C_FREQ_MODE_STANDARD 4,
dev-virtbase + I2C_CR);
}
- writel(dev-cfg.sm 4, dev-virtbase + I2C_CR);
+ writel(dev-sm 4, dev-virtbase + I2C_CR);
/* set the Tx and Rx FIFO threshold */
- writel(dev-cfg.tft, dev-virtbase + I2C_TFTR);
- writel(dev-cfg.rft, dev-virtbase + I2C_RFTR);
+ writel(dev-tft, dev-virtbase + I2C_TFTR);
+ writel(dev-rft, dev-virtbase + I2C_RFTR);
}
/**
@@ -958,61 +950,35 @@ static const struct i2c_algorithm nmk_i2c_algo = {
.functionality = nmk_i2c_functionality
};
-static struct nmk_i2c_controller u8500_i2c = {
- .tft= 1, /* Tx FIFO threshold */
- .rft= 8, /* Rx FIFO threshold */
- .clk_freq = 40, /* fast mode operation */
- .timeout
Re: [PATCH 4/9] i2c: i2c-stu300: replace platform_driver_probe to support deferred probing
On Tue, Oct 8, 2013 at 10:35 PM, Wolfram Sang w...@the-dreams.de wrote: Subsystems like pinctrl and gpio rightfully make use of deferred probing at core level. Now, deferred drivers won't be retried if they don't have a .probe function specified in the driver struct. Fix this driver to have that, so the devices it supports won't get lost in a deferred probe. Signed-off-by: Wolfram Sang w...@the-dreams.de Cc: Linus Walleij linus.wall...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org 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 2/9] irqdomain: Introduce __irq_create_mapping()
On Tue, Sep 24, 2013 at 8:28 PM, Thierry Reding thierry.red...@gmail.com wrote: Given that linux-next might not be with us for much longer before the 3.12 release, I'm thinking of deferring the series until then. Or at least trying to get it merged. Otherwise we'll probably have to deal with a lot of fall out during the merge window. Hm, how unfortunate. Typically this is the kind of topic branch that should go in separately to linux-next. In any case, it'd be nice to get some feedback on the general idea of the patch series from other people involved. I'd hate to do all the conversions just to have it NAKed at the last minute. With the direction we've discussed here atleast I won't be doing any NACKing if it's of any help... 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 2/9] irqdomain: Introduce __irq_create_mapping()
On Mon, Sep 23, 2013 at 10:29 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 23, 2013 at 09:14:30PM +0200, Linus Walleij wrote: I think it is better to first go over the call sites and make them all handle negative return numbers rather than pushing the obscure __interface. (...) Well, the problem is that the current patch changes the signature of the function as well, therefore the call sites will have to be updated all at once in a single patch to avoid build breakage. Hm yeah OK I see the problem, but can we atleast avoid the __thing? Like calling the new function irq_create_mapping_strict() or whatever. Another alternative could be to change the signature in a way that does not break compatibility. For instance I think it could work out if we change this function to return int instead of unsigned int but keep the same semantics to begin with (return 0 on failure). Then update all call sites to handle potential negative errors and after that return negative error codes. Hm that sounds like an attractive solution to me actually. That still wouldn't catch any callers introduced between the patch creation and application. Such things happen all the time, just have to be attentive in what goes into linux-next... Another minor thing: +static int __irq_create_mapping(struct irq_domain *domain, + irq_hw_number_t hwirq, unsigned int *virqp) Unless you can make a very good case for why there should be a v in the beginning of virqp, then remove it and call it irqp simply. All Linux IRQs are virtual and we're already clearly separating out those that are not by calling them hwirq. 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 2/9] irqdomain: Introduce __irq_create_mapping()
On Mon, Sep 16, 2013 at 10:31 AM, Thierry Reding thierry.red...@gmail.com wrote: This is a version of irq_create_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_mapping() as a wrapper around the new __irq_create_mapping(). Signed-off-by: Thierry Reding tred...@nvidia.com Surprise! I don't like this. I think it is better to first go over the call sites and make them all handle negative return numbers rather than pushing the obscure __interface. I know from patch 0 that you think it's too much to change these 127 call sites but I don't think so, and I'm happy to merge one big patch changing all the 20 users in drivers/gpio. Likewise with the 11 consumers in drivers/pinctrl. It's just a a few archs+subsystems and it's just plain work. So do that first. 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 3/9] irqdomain: Introduce __irq_create_of_mapping()
On Mon, Sep 16, 2013 at 11:17 PM, Rob Herring robherri...@gmail.com wrote: 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. I second this and also don't want the first patch to use a wrapper. 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 5/9] of/irq: Introduce __of_irq_to_resource()
On Mon, Sep 16, 2013 at 11:29 PM, Rob Herring robherri...@gmail.com wrote: 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. I second this comment. 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 4/9] of/irq: Introduce of_irq_get()
On Tue, Sep 17, 2013 at 3:28 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 16, 2013 at 04:24:47PM -0500, Rob Herring wrote: *_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. Okay, I'll make the new function __irq_of_parse_and_map(). I don't know why i detest __prefixing so much but I think it's really nasty. Usually this is reserved for compiler- and linker related things, like __packed; or __init. I would prefer irq_of_parse_and_map_strict() or something like that. 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 9/9] gpio: tegra: Use module_platform_driver()
On Mon, Sep 16, 2013 at 10:32 AM, Thierry Reding thierry.red...@gmail.com wrote: With the driver core now resolving interrupt references at probe time, it is no longer necessary to force explicit probe ordering using initcalls. Signed-off-by: Thierry Reding tred...@nvidia.com --- Note that there are potentially many more drivers that can be switched to the generic module_*_driver() interfaces now that interrupts can be resolved later and deferred probe should be able to handle all the ordering issues. Let me see if I get this right: so this would be all GPIO/pinctrl drivers which are probed exclusively from the device tree so anything that depends explicitly on CONFIG_OF would be a candidate? I think we have a bit of a problem that some drivers depend only on a certain arch or compiles directly for a certain arch without any specific Kconfig option so that this may be a bit hard to spot, so we should keep an eye out for this once this probing scheme is in place. 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 1/4 v2] mfd: add STw481x driver
On Mon, Sep 16, 2013 at 12:40 PM, Mark Brown broo...@kernel.org wrote: On Mon, Sep 16, 2013 at 10:19:56AM +0100, Lee Jones wrote: Can't you just NULL .id_table? No. That is not OK with the I2C core. It's not easy to get rid of the requirement for .id_table not to be NULL either. Here's the code which would use it: /* match on an id table if there is one */ if (driver-id_table) return i2c_match_id(driver-id_table, client) != NULL; Matching for dummy will just waste cycles. i2c_device_probe() will return -ENODEV if id_table is NULL before we get to actually matching. We could remove that check though... Copy+pasting from my own reply earlier: I've tried to fix this for DT-only I2C devices and this very driver was the reason. But a tiresome regression due to drivers relying on this i2c_device_id not being NULL and inability to remove it from the I2C core without refactoring the world ensued, see: commit c80f52847c50109ca248c22efbf71ff10553dca4 Reverted in: commit 661f6c1cd926c6c973e03c6b5151d161f3a666ed For this reason I think: http://marc.info/?l=linux-nextm=137148411231784w=2 I have tentatively given up getting pure DT I2C drivers to probe, I don't think I have the whole picture, but Wolfram has serious doubts about this and say we have to be careful Wolfram, do you have some ideas on how we should proceed or ar you happy with merging this as-is? 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: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
On Wed, Aug 28, 2013 at 11:57 AM, Wolfram Sang w...@the-dreams.de wrote: On Tue, Aug 20, 2013 at 04:32:28PM +0800, Zhangfei Gao wrote: Instead of use platform_driver_probe, use module_platform_driver To support deferred probing Also subsys_initcall may too early to auto set pinctl Signed-off-by: Zhangfei Gao zhangfei@linaro.org Acked-by: Baruch Siach bar...@tkos.co.il This patch is tougher than it looks. You need it, because subsys_initcall may be too early for pinctrl. pinctrl is initialized very early, core_initcall(). This is more a question of individual pin control drivers and when they probe, and dependencies trying to take a pinctrl handle before the pin controller is available will be deferred. Even by those grabbed in the core by drivers/base/pinctrl.c. 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: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
On Thu, Aug 29, 2013 at 12:55 PM, zhangfei gao zhangfei@gmail.com wrote: Besides, also find platform_driver_probe is used in drivers/i2c/busses/i2c-imx.c and drivers/i2c/busses/i2c-stu300.c. The platform_driver_probe() is basically a footprint optimization (more code can be discarded after boot) and I'm happy to patch it if it disturbs anything, it is *really* not important for this driver. Do you guys need a low footprint? Else there is no use to have platform_driver_probe() in there. 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 06/27] drivers/i2c/busses: don't check resource with devm_ioremap_resource
On Tue, Jul 23, 2013 at 8:01 PM, Wolfram Sang w...@the-dreams.de wrote: devm_ioremap_resource does sanity checks on the given resource. No need to duplicate this in the driver. Signed-off-by: Wolfram Sang w...@the-dreams.de --- Please apply via the subsystem-tree. Are you talking to yourself ;-) Acked-by: Linus Walleij linus.wall...@linaro.org 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 v2] i2c: core: make it possible to match a pure device tree driver
On Tue, Jun 18, 2013 at 9:33 AM, Wolfram Sang w...@the-dreams.de wrote: On Mon, Jun 17, 2013 at 11:15:30PM +0100, Grant Likely wrote: On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij linus.wall...@linaro.org wrote: OK that works for me, I'm not in any hurry. Deferring by a merge window isn't going to make it any less painful. Do your best to find all the users that need to be changed. Use a coccinelle search perhaps, but I think it should be merged anyway. I'll try a bit of my coccinelle-foo today and then decide. Thanks Wolfram, much appreciated. 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 0/2] Fix kernel panics with certain I2C tps6* chips
On Mon, Jun 17, 2013 at 7:47 PM, Tuomas Tynkkynen ttynkky...@nvidia.com wrote: Hi, Latest linux-next head (next-20130617) seems to have some backwards-incompatible changes to the i2c core, which breaks the tps* drivers in our boards and cause panics on boot. You should probably put Wolfram (the i2c maintainer) on the To: line. Tuomas Tynkkynen (2): mfd: tps65910: Fix crash in i2c_driver .probe regulator: tps62360: Fix crash in i2c_driver .probe Nice :-) 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 v2] i2c: core: make it possible to match a pure device tree driver
On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren swar...@wwwdotorg.org wrote: This has just shown up in next-20130617, and breaks at least the TPS65910 and TPS62360 drivers, since they assume that the id parameter passed to probe is non-NULL. However, now the parameter is NULL since these drivers have both an ID table and an OF match table. So you mean they come in through the DT boot path and assume that parameter is non-null even though they should not make use of it? I'd like to suggest this patch be reverted an re-introduced immediately after the merge window. That should give enough time for everyone to get a heads-up on fixing any drivers with a similar problem, rather than trying to cram all that in immediately before the merge window. OK that works for me, I'm not in any hurry. Wolfram get to decide how to handle this... 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 v2] i2c: core: make it possible to match a pure device tree driver
On Wed, Jun 12, 2013 at 3:20 PM, Grant Likely grant.lik...@linaro.org wrote: On Fri, 7 Jun 2013 23:32:42 +0200, Wolfram Sang w...@the-dreams.de wrote: I guess your solution is the least intrusive one. Still, it could happen that of_match_table is scanned three times (by driver core, i2c layer, and i2c driver) which is IMO an indication to look for a more elegant solution tp find out what really matched? It's what we do on platform_devices. It really isn't an expensive operation so I haven't pushed anyone to go optimize it. I tried to think of something and it ended up with ideas like decorating the device tree representation and it was just ... ouch. 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 v2] i2c: core: make it possible to match a pure device tree driver
On Fri, Jun 7, 2013 at 11:32 PM, Wolfram Sang w...@the-dreams.de wrote: ... I2C devices probed from device tree should subsequently be fixed to handle the case where of_match_table() is used (I think none of them do that today), and platforms should fix their device trees to use compatible strings for I2C devices instead of setting the name to Linux device driver names as is done in multiple cases today. I guess your solution is the least intrusive one. Still, it could happen that of_match_table is scanned three times (by driver core, i2c layer, and i2c driver) which is IMO an indication to look for a more elegant solution tp find out what really matched? I think that is a generic problem with the device tree being completely stateless, and rather a comment on the of_match_device() intrinsics being inelegant than on this patch? Do you see it as a blocker for the patch? What happens before this patch is that instead of looping over the of_match_table, the id_table is always iterated to the end also in the DT case, yielding NULL as the last argument to driver-probe() anyway. Maybe the OF people have some comments on this... I cannot really see how it could be any different given the way the FDT works :-/ 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
[PATCH] i2c: nomadik: support elder Nomadiks
The Nomadik I2C block was introduced with the Nomadik STn8815
SoC (the STn8810 incidentally is identical to the one named
i2c-stu300.c). However as developments have only been tested
on the DB8500 family, it was not properly working with the
STn8815 anymore.
Rectify this by adding some vendor variant data in the same
manner as other PrimeCells, and switch code path depending
on version.
Tested on the S8815 Nomadik dongle.
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/i2c/busses/i2c-nomadik.c | 43 ++--
1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 37353b3..f6d18ce 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -106,6 +106,16 @@
/* maximum threshold value */
#define MAX_I2C_FIFO_THRESHOLD 15
+/**
+ * struct i2c_vendor_data - per-vendor variations
+ * @has_mtdws: variant has the MTDWS bit
+ * @fifodepth: variant FIFO depth
+ */
+struct i2c_vendor_data {
+ bool has_mtdws;
+ u32 fifodepth;
+};
+
enum i2c_status {
I2C_NOP,
I2C_ON_GOING,
@@ -138,6 +148,7 @@ struct i2c_nmk_client {
/**
* struct nmk_i2c_dev - private data structure of the controller.
+ * @vendor: vendor data for this variant.
* @adev: parent amba device.
* @adap: corresponding I2C adapter.
* @irq: interrupt line for the controller.
@@ -155,6 +166,7 @@ struct i2c_nmk_client {
* @busy: Busy doing transfer.
*/
struct nmk_i2c_dev {
+ struct i2c_vendor_data *vendor;
struct amba_device *adev;
struct i2c_adapter adap;
int irq;
@@ -431,7 +443,7 @@ static int read_i2c(struct nmk_i2c_dev *dev, u16 flags)
irq_mask = (I2C_IT_RXFNF | I2C_IT_RXFF |
I2C_IT_MAL | I2C_IT_BERR);
- if (dev-stop)
+ if (dev-stop || !dev-vendor-has_mtdws)
irq_mask |= I2C_IT_MTD;
else
irq_mask |= I2C_IT_MTDWS;
@@ -511,7 +523,7 @@ static int write_i2c(struct nmk_i2c_dev *dev, u16 flags)
* set the MTDWS bit (Master Transaction Done Without Stop)
* to start repeated start operation
*/
- if (dev-stop)
+ if (dev-stop || !dev-vendor-has_mtdws)
irq_mask |= I2C_IT_MTD;
else
irq_mask |= I2C_IT_MTDWS;
@@ -978,6 +990,8 @@ static int nmk_i2c_probe(struct amba_device *adev, const
struct amba_id *id)
struct device_node *np = adev-dev.of_node;
struct nmk_i2c_dev *dev;
struct i2c_adapter *adap;
+ struct i2c_vendor_data *vendor = id-data;
+ u32 max_fifo_threshold = (vendor-fifodepth / 2) - 1;
if (!pdata) {
if (np) {
@@ -994,12 +1008,25 @@ static int nmk_i2c_probe(struct amba_device *adev, const
struct amba_id *id)
pdata = u8500_i2c;
}
+ if (pdata-tft max_fifo_threshold) {
+ dev_warn(adev-dev, requested TX FIFO threshold %u, adjusted
down to %u\n,
+ pdata-tft, max_fifo_threshold);
+ pdata-tft = max_fifo_threshold;
+ }
+
+ if (pdata-rft max_fifo_threshold) {
+ dev_warn(adev-dev, requested RX FIFO threshold %u, adjusted
down to %u\n,
+ pdata-rft, max_fifo_threshold);
+ pdata-rft = max_fifo_threshold;
+ }
+
dev = kzalloc(sizeof(struct nmk_i2c_dev), GFP_KERNEL);
if (!dev) {
dev_err(adev-dev, cannot allocate memory\n);
ret = -ENOMEM;
goto err_no_mem;
}
+ dev-vendor = vendor;
dev-busy = false;
dev-adev = adev;
amba_set_drvdata(adev, dev);
@@ -1134,14 +1161,26 @@ static int nmk_i2c_remove(struct amba_device *adev)
return 0;
}
+static struct i2c_vendor_data vendor_stn8815 = {
+ .has_mtdws = false,
+ .fifodepth = 16, /* Guessed from TFTR/RFTR = 7 */
+};
+
+static struct i2c_vendor_data vendor_db8500 = {
+ .has_mtdws = true,
+ .fifodepth = 32, /* Guessed from TFTR/RFTR = 15 */
+};
+
static struct amba_id nmk_i2c_ids[] = {
{
.id = 0x00180024,
.mask = 0x00ff,
+ .data = vendor_stn8815,
},
{
.id = 0x00380024,
.mask = 0x00ff,
+ .data = vendor_db8500,
},
{},
};
--
1.8.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
[PATCH] i2c: nomadik: allocate adapter number dynamically
The Nomadik I2C was using a local atomic counter to number
the I2C adapters. This does not work on configurations where
you also add, say a GPIO bit-banged adapter to the system.
They will start to conflict about being adapter 0.
There is no reason to use the numbered adapter function, and
the semantic effect on systems with only Nomadik I2C blocks
will be none - instead of increasing the number atomically
in the driver itself, it is done in the I2C core.
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/i2c/busses/i2c-nomadik.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 9f1423a..96c8515 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -15,7 +15,6 @@
#include linux/init.h
#include linux/module.h
#include linux/amba/bus.h
-#include linux/atomic.h
#include linux/slab.h
#include linux/interrupt.h
#include linux/i2c.h
@@ -981,8 +980,6 @@ static void nmk_i2c_of_probe(struct device_node *np,
pdata-sm = I2C_FREQ_MODE_FAST;
}
-static atomic_t adapter_id = ATOMIC_INIT(0);
-
static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
{
int ret = 0;
@@ -1095,10 +1092,9 @@ static int nmk_i2c_probe(struct amba_device *adev, const
struct amba_id *id)
adap-class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
adap-algo = nmk_i2c_algo;
adap-timeout = msecs_to_jiffies(pdata-timeout);
- adap-nr = atomic_read(adapter_id);
+ adap-nr = -1;
snprintf(adap-name, sizeof(adap-name),
-Nomadik I2C%d at %pR, adap-nr, adev-res);
- atomic_inc(adapter_id);
+Nomadik I2C at %pR, adev-res);
/* fetch the controller configuration from machine */
dev-cfg.clk_freq = pdata-clk_freq;
@@ -1113,7 +1109,7 @@ static int nmk_i2c_probe(struct amba_device *adev, const
struct amba_id *id)
initialize %s on virtual base %p\n,
adap-name, dev-virtbase);
- ret = i2c_add_numbered_adapter(adap);
+ ret = i2c_add_adapter(adap);
if (ret) {
dev_err(adev-dev, failed to add adapter\n);
goto err_add_adap;
--
1.8.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 11/11] i2c: omap: enhance pinctrl support
On Fri, May 31, 2013 at 8:07 PM, Kevin Hilman khil...@linaro.org wrote: But that brings up a bigger question about whether or not we should be doing the rest of this (idle/sleep) pin management in the drivers or in the driver core as well. I would much prefer it be handled by the driver core. Yes. See my suggestion in 2/11. In fact, since these are all PM related events, it should probably be handled by the PM core and seems pretty straight forward to do so. I can see a clean interface with three simple functions toward pinctrl/consumer.h for switching between the default, idle and sleep states, but you may see even further... 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 v2] i2c: core: make it possible to match a pure device tree driver
On Wed, May 22, 2013 at 9:56 AM, Linus Walleij linus.wall...@linaro.org wrote: On Mon, May 13, 2013 at 10:18 PM, Linus Walleij linus.wall...@linaro.org wrote: This tries to address an issue found when writing an MFD driver for the Nomadik STw481x PMICs: as the platform is using device tree exclusively I want to specify the driver matching like this: (...) --- ChangeLog v1-v2: - Use of_match_device() to determine if there is a DT match in the probe code. If there is a match we pass NULL for the id_table match parameter. Ping on this. v2 should be doing what Grant suggested... Ping, nocheinmal. 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 v2] i2c: core: make it possible to match a pure device tree driver
On Mon, May 13, 2013 at 10:18 PM, Linus Walleij linus.wall...@linaro.org wrote: This tries to address an issue found when writing an MFD driver for the Nomadik STw481x PMICs: as the platform is using device tree exclusively I want to specify the driver matching like this: (...) --- ChangeLog v1-v2: - Use of_match_device() to determine if there is a DT match in the probe code. If there is a match we pass NULL for the id_table match parameter. Ping on this. v2 should be doing what Grant suggested... 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] i2c: core: make it possible to match a pure device tree driver
On Mon, May 13, 2013 at 9:16 AM, Sascha Hauer s.ha...@pengutronix.de wrote: - status = driver-probe(client, i2c_match_id(driver-id_table, client)); + if (dev-driver-of_match_table) + /* Device tree matching */ + status = driver-probe(client, NULL); + else + /* Fall back to matching the id_table */ + status = driver-probe(client, i2c_match_id(driver-id_table, client)); If you correctly register a device with vendor,product in the devicetree the driver can already fetch the of_device_id using of_match_device(dt_ids, client-dev) just like a platform driver would do aswell. Yes, this is what I write in the commit message: (...) If the driver wants to deduce secondary info from the struct of_device_id .data field, it has to call of_match_device() on its own match table in the probe function device tree probe path. i2c_match_id will return a NULL pointer if called with vendor,product, because nothing matches in the drivers id_table, so for this case you change nothing. If anything, you introduce the problem that a devicetree capable driver no longer gets a i2c_device_id if the device was instantiated with i2c_board_info. Hm, you're right there, what about this: + if (dev-of_node) + /* Device tree matching */ + status = driver-probe(client, NULL); + else + /* Fall back to matching the id_table */ + status = driver-probe(client, i2c_match_id(driver-id_table, client)); If the device has an of_node it surely should not be using the id_table and it'd be correct to pass NULL, right? 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
[PATCH] i2c: core: make it possible to match a pure device tree driver
This tries to address an issue found when writing an MFD driver
for the Nomadik STw481x PMICs: as the platform is using device
tree exclusively I want to specify the driver matching like
this:
static const struct of_device_id stw481x_match[] = {
{ .compatible = st,stw4810, },
{ .compatible = st,stw4811, },
{},
};
static struct i2c_driver stw481x_driver = {
.driver = {
.name = stw481x,
.of_match_table = stw481x_match,
},
.probe = stw481x_probe,
.remove = stw481x_remove,
};
However that turns out not to be possible: the I2C probe code
is written so that the probe() call is always passed a match
from i2c_match_id() using non-devicetree matches.
This is probably why most devices using device tree for I2C
clients currently will pass no .of_match_table *at all* but
instead just use .id_table from struct i2c_driver to match
the device. As you realize that means that the whole idea with
compatible strings is discarded, and that is why we find strange
device tree I2C device compatible strings like product instead
of vendor,product as you could expect.
Let's figure out how to fix this before the mess spreads. This
patch will allow probeing devices with only an of_match_table
as per above, and will pass NULL as the second argument to the
probe() function. If the driver wants to deduce secondary info
from the struct of_device_id .data field, it has to call
of_match_device() on its own match table in the probe function
device tree probe path.
If drivers define both an .of_match_table *AND* a i2c_driver
.id_table, the .of_match_table will take precedence, just
as is done in the i2c_device_match() function in i2c-core.c.
I2C devices probed from device tree should subsequently be
fixed to handle the case where of_match_table() is
used (I think none of them do that today), and platforms should
fix their device trees to use compatible strings for I2C devices
instead of setting the name to Linux device driver names as is
done in multiple cases today.
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Grant Likely grant.lik...@linaro.org
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
I would need some device tree core people to confirm that I am
on the right track with this. I was s confused when I found
that .of_match_table could not be used with I2C devices...
---
drivers/i2c/i2c-core.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 6b63cc7..30b5bb2 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -240,7 +240,7 @@ static int i2c_device_probe(struct device *dev)
return 0;
driver = to_i2c_driver(dev-driver);
- if (!driver-probe || !driver-id_table)
+ if (!driver-probe || (!driver-id_table
!dev-driver-of_match_table))
return -ENODEV;
client-driver = driver;
if (!device_can_wakeup(client-dev))
@@ -248,7 +248,12 @@ static int i2c_device_probe(struct device *dev)
client-flags I2C_CLIENT_WAKE);
dev_dbg(dev, probe\n);
- status = driver-probe(client, i2c_match_id(driver-id_table, client));
+ if (dev-driver-of_match_table)
+ /* Device tree matching */
+ status = driver-probe(client, NULL);
+ else
+ /* Fall back to matching the id_table */
+ status = driver-probe(client, i2c_match_id(driver-id_table,
client));
if (status) {
client-driver = NULL;
i2c_set_clientdata(client, NULL);
--
1.8.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 3/4] gpio: Kontron PLD gpio driver
On Fri, Apr 12, 2013 at 1:09 PM, Michael Brunner mi...@gmx.de wrote:
(...)
+struct kempld_gpio_data {
+ struct gpio_chipchip;
+ int irq;
+ struct kempld_device_data *pld;
+ uint16_tmask;
Just u16?
The specification allows 16 GPIOs for this device, therefore this seems
to be the right size. Would it be better to use another type instead?
Ah, I was just asking you to use u16 instead of uint16_t.
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 3/4] gpio: Kontron PLD gpio driver
On Tue, Apr 9, 2013 at 6:41 PM, Guenter Roeck li...@roeck-us.net wrote: On Tue, Apr 09, 2013 at 10:46:15AM +0200, Linus Walleij wrote: On Mon, Apr 8, 2013 at 7:15 PM, Kevin Strasser kevin.stras...@linux.intel.com wrote: From: Michael Brunner michael.brun...@kontron.com Add gpio support for the on-board PLD found on some Kontron embedded modules. Signed-off-by: Michael Brunner michael.brun...@kontron.com Signed-off-by: Kevin Strasser kevin.stras...@linux.intel.com This looks very generic, setting and clearing bits in bytesized registers. Can you please attempt to use generic GPIO for this? Linus, I looked into it, but for my part I seem to be missing how the generic GPIO code permits locking access to the hardware (PLD) and setting the PLD's page register. In other words, I don't immediately see how to call kempld_get_mutex_set_index() from the generic GPIO code. The other drivers using generic GPIO code don't seem to have that requirement. Ah yes, I was totally wrong here. I thought it was MMIO while it is indeed through an MFD proxy. I'll have a second look then... 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 3/4] gpio: Kontron PLD gpio driver
On Mon, Apr 8, 2013 at 7:15 PM, Kevin Strasser
kevin.stras...@linux.intel.com wrote:
From: Michael Brunner michael.brun...@kontron.com
Add gpio support for the on-board PLD found on some Kontron embedded
modules.
Signed-off-by: Michael Brunner michael.brun...@kontron.com
Signed-off-by: Kevin Strasser kevin.stras...@linux.intel.com
Trying to do some real review...
(...)
+++ b/drivers/gpio/gpio-kempld.c
+#include linux/acpi.h
Is this used?
+#include linux/platform_device.h
+#include linux/gpio.h
+#include linux/mfd/kempld.h
+#include linux/seq_file.h
+
+#include gpio-kempld.h
+
+static int gpiobase = -1;
+static int gpioien = 0x00;
+static int gpioevt_lvl_edge = -1;
+static int gpioevt_low_high = -1;
+static int gpionmien = 0x00;
(...)
+static int kempld_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ struct kempld_gpio_data *gpio
+ = container_of(chip, struct kempld_gpio_data, chip);
+ return gpio-irq;
+}
I don't understand this *at all* so help me out here.
.gpio_to_irq() should return a *Linux* IRQ number, usually we take
the event offset (in this case) and map to a Linux IRQ using the
irqdomain helper library. Can you explain how we can be sure that
this number (apparently just a read from a register on the device)
can be made to correspond to a Linux IRQ?
Also if this thing can generate IRQs, are these one line to the CPU
per IRQ really? Don't you need to demux the status register and
create a cascades irqchip?
Maybe it's just me not understanding x86 ACPI so bear with me...
+static int kempld_gpio_setup_event(struct kempld_gpio_data *gpio)
+{
+ struct kempld_device_data *pld = gpio-pld;
+ struct gpio_chip *chip = gpio-chip;
+ int irq;
+
+ irq = gpio-irq;
+
+ kempld_get_mutex_set_index(pld, KEMPLD_IRQ_GPIO);
+ irq = kempld_read8(pld, KEMPLD_IRQ_GPIO);
+
+ /* Leave if interrupts are not supported by the GPIO core */
+ if ((irq 0xf0) == 0xf0)
+ return 0;
+
+ gpio-irq = irq 0x0f;
So you read the IRQ from some plug-n-play here, and it's some
system-wide IRQ number?
(...)
+ if (gpio-irq)
+ chip-to_irq = kempld_gpio_to_irq;
So that is this mystery with the IRQs and how they turn into
Linux IRQs.
+module_param(gpiobase, int, 0444);
Why do you need to be able to configure this?
It must be a real usecase, debugging can be done by patching
the code.
+module_param(gpioien, int, 0444);
+module_param(gpioevt_lvl_edge, int, 0444);
+module_param(gpioevt_low_high, int, 0444);
+module_param(gpionmien, int, 0444);
Argh how can anyone possibly make this out ... do you really
need them or can we get rid of some and rely on autodetect?
+MODULE_DESCRIPTION(KEM PLD GPIO Driver);
+MODULE_AUTHOR(Michael Brunner michael.brun...@kontron.com);
+MODULE_LICENSE(GPL);
+MODULE_ALIAS(platform:kempld_gpio);
+MODULE_PARM_DESC(gpiobase, Set GPIO base (default -1=dynamic));
+MODULE_PARM_DESC(gpioien, Set GPIO IEN register (default 0x00));
+MODULE_PARM_DESC(gpioevt_lvl_edge,
+ Set GPIO EVT_LVL_EDGE register (default -1=no
change));
+MODULE_PARM_DESC(gpioevt_low_high,
+ Set GPIO EVT_LOW_HIGH register (default -1=no
change));
+MODULE_PARM_DESC(gpionmien, Set GPIO NMIEN register (default 0x00));
So I don't really like that interrupt enablement and edge and low/high
is done with module parameters instead of just creating an irqchip and
have it implement the operations to do exactly these things at runtime
instead.
Again maybe some x86 thing I don't get...
diff --git a/drivers/gpio/gpio-kempld.h b/drivers/gpio/gpio-kempld.h
(...)
+struct kempld_gpio_data {
+ struct gpio_chipchip;
+ int irq;
+ struct kempld_device_data *pld;
+ uint16_tmask;
Just u16?
+};
(...)
diff --git a/drivers/gpio/gpio-kempld_now1.c b/drivers/gpio/gpio-kempld_now1.c
+#include linux/io.h
Do you use this?
+#include linux/slab.h
+#include linux/errno.h
+#include linux/acpi.h
And this?
+#include linux/platform_device.h
+#include linux/gpio.h
+#include linux/mfd/kempld.h
+#include linux/seq_file.h
+
+#include gpio-kempld.h
(...)
+
Most comments concern the other driver too.
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] i2c: sirf: get the i2c pin group by pinctrl api
On Mon, Mar 18, 2013 at 8:22 AM, Barry Song barry.s...@csr.com wrote: From: Barry Song baohua.s...@csr.com hardcode set i2c pin group to i2c function before, here we move to use standard pinctrl API to get pins of the group. Signed-off-by: Barry Song baohua.s...@csr.com Cc: Linus Walleij linus.wall...@linaro.org NAK. This is done by the device core as of commit ab78029ecc347debbd737f06688d788bd9d60c1d drivers/pinctrl: grab default handles from device core. You only need to fetch pinctrl handles in drivers if you're using anything else than the default state. 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] i2c: sirf: get the i2c pin group by pinctrl api
On Wed, Mar 27, 2013 at 11:36 AM, Barry Song 21cn...@gmail.com wrote: 2013/3/27 Linus Walleij linus.wall...@linaro.org: You only need to fetch pinctrl handles in drivers if you're using anything else than the default state. well. missed this recent commit. it should mean we hould drop all devm_pinctrl_get_select_default() if the driver only takes the default pinctrl? Yes. it looks like there are still a lot of drivers doing that. anyway, i will drop them in SiRFsoc drivers. and involve you in the coming patch. I am meaning to fix that up. Just haven't had time to... Prior to that patch it was the right thing to do. 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] i2c: s3c24xx: allow device core to setup default pin configuration
On Mon, Mar 4, 2013 at 2:42 PM, Thomas Abraham thomas.abra...@linaro.org wrote: With device core now able to setup the default pin configuration, the call to devm_pinctrl_get_select_default can be removed. And the pin configuration code based on the deprecated Samsung specific gpio bindings is also removed. Signed-off-by: Thomas Abraham thomas.abra...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org 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] i2c: s3c2410: check for NULL pinctrl handle
On Sun, Feb 24, 2013 at 6:00 PM, Tomasz Figa tomasz.f...@gmail.com wrote: Note that we are talking here about a temporary solution. The legacy DT- based pin configuration will go away after all the DT-enabled platforms using this driver get migrated to pin control and so will the need to check if pin control is available. So use AUXDATA, and you get a pdata for that driver? Hmm, and then have some platform data passed statically and some parsed from device tree? This is done by several in-tree drivers today. It is even necessary for things like machine-specific callbacks. Not even saying that we are going towards getting rid of auxdata, not adding further dependencies for it. The other option is to do the non-temporary solution you are referring to below... Sorry, but this sounds more broken to me than checking the return value of devm_pinctrl_get_select_default for NULL in the driver. Both are bad solution, auxdata is less bad than trying to check struct pinctrl * handles for non-NULL, which has *never* been a good thing to do and should never have been merged in the first place. (Maybe I ACKed that, then I was doing something stupid.) Still, all the platforms relying on the legacy DT GPIO support should have been already migrated to pin control, so ideally instead of fixing the drivers to continue supporting the deprecated method, such platforms should be fixed. I agree. 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] pinctrl: return real error codes when pinctrl is not included
On Sun, Feb 24, 2013 at 11:34 PM, Heiko Stübner he...@sntech.de wrote: [Me] This make me suspect that you have this ugly patch in some private repo and I will be seeing it again and again :-( All my s3c24xx work is done is my spare time, so I have to confess I came up with this ugly patch all by myself when working on dt support for my machine and stumbling upon the described problem with the pin configuration. So, as it is obviously wrong, I also won't hold onto it. In any case, thanks for the thorough explanation, which I probably won't forget anytime soon. Hm, maybe I have come across as too harsh and then I feel bad about that :-( I really want spare-time contributors, they are often more valueable in addition to the corporate ones since they provide an outside view of things. 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] i2c: s3c2410: check for NULL pinctrl handle
On Sat, Feb 23, 2013 at 6:57 PM, Heiko Stübner he...@sntech.de wrote: When pinctrl is not built the fallback functions fail silently and emit either 0 error codes or NULL pinctrl handles. Therefore it's needed to also check for this NULL-handle when falling back to parsing the i2c gpios from devicetree. Signed-off-by: Heiko Stuebner he...@sntech.de NAK. This is not the right solution for this driver. It uses pinctrl in a very simplistic way, just grabbing the default handler. After commit ab78029ecc347debbd737f06688d788bd9d60c1d: drivers/pinctrl: grab default handles from device core The right solution is to simply revert commit 2693ac69880a33d4d9df6f128415b65e745f00ba i2c: s3c2410: Add support for pinctrl Tomasz are you OK with this, or will you add more fine-grained pinctrl (like runtime PM etc) to this driver? 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] pinctrl: return real error codes when pinctrl is not included
to do it some other way. 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] i2c: s3c2410: check for NULL pinctrl handle
On Sun, Feb 24, 2013 at 1:38 AM, Tomasz Figa tomasz.f...@gmail.com wrote:
The driver must know whether pin control is available, because it has to
fall back to legacy GPIO-based pin configuration if it is not. This means
that we must either check for NULL (which probably is not right, since
returned handle is considered to be opaque) or pin control core must
return an error code specific to this situation, e.g. -ENODEV.
OK so pass a flag like a bool in your platform data from the
machine like go into linux/platform_data/i2c-s3c2410.h
and add:
struct s3c2410_platform_i2c {
bool use_that_old_gpio_interface;
(...)
};
Instead of trying to semi-guess if the pinctrl framework is there?
Surely you know this when setting up the pdata from your machine?
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] i2c: s3c2410: check for NULL pinctrl handle
On Sun, Feb 24, 2013 at 1:58 AM, Tomasz Figa tomasz.f...@gmail.com wrote: [Me] Surely you know this when setting up the pdata from your machine? Cases 2) and 3) are both DT-enabled cases, where there is no pdata coming from board-specific code. (...) Note that we are talking here about a temporary solution. The legacy DT- based pin configuration will go away after all the DT-enabled platforms using this driver get migrated to pin control and so will the need to check if pin control is available. So use AUXDATA, and you get a pdata for that driver? 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 RFC 1/1] gpio: mcp23s08: convert driver to DT
On Wed, Feb 6, 2013 at 10:31 AM, Lars Poeschel poesc...@lemonage.de wrote: The thing that confused me was, that the platform_data for the chip has a mandatory base member, that sets the linux global gpio number at which the chip should appear. Yes this is common. I think you should look at other drivers under drivers/gpio using device tree, and how they work around this. As stated, as a last resort you can use AUXDATA to anyway assign a piece of platform data per instance. In the Nomadik driver, we use the block instance ID and multiply by a factor of the numbers of GPIOs on each instance. And luckily the base is zero. Not elegant maybe, but the global GPIO numberspace is not elegant by nature. 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 RFC 1/1] gpio: mcp23s08: convert driver to DT
On Thu, Jan 31, 2013 at 4:58 PM, Lars Poeschel la...@wh2.tu-dresden.de wrote: --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt @@ -0,0 +1,27 @@ +Microchip MCP2308/MCP23S08/MCP23017/MCP23S17 driver for +8-/16-bit I/O expander with serial interface (I2C/SPI) + +Required properties: +- compatible : Should be mcp,mcp23s08-gpio, mcp,mcp23s17-gpio, + mcp,mcp23008-gpio or mcp,mcp23017-gpio +- base : The first gpio number that should be assigned by this chip. No. We do not tie the global GPIO numbers into the device tree. In the DT GPIOs are referenced by ampersand gpio0 1 2 notation referring to the instance, so as you realize DT itself has no need for that number. Further it is not OS-neutral. You have to find another way to handle this in the driver code. In worst case: use AUXDATA. 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
[PATCH v5] i2c: nomadik: adopt pinctrl support
From: Patrice Chotard patrice.chot...@stericsson.com
Amend the I2C nomadik pin controller to optionally take a pin control
handle and set the state of the pins to:
- default on boot, resume and before performing an i2c transfer
- idle after initial default, after resume default, and after each
i2c xfer
- sleep on suspend()
This should make it possible to optimize energy usage for the pins
both for the suspend/resume cycle, and for runtime cases inbetween
I2C transfers.
Signed-off-by: Patrice Chotard patrice.chot...@stericsson.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
ChangeLog v4-v5:
- Fix some coding style issues pointed out by Wolfram.
ChangeLog v3-v4:
- Rebase onto v3.8-rc2
ChangeLog v2-v3:
- Rebase on top of the patch from Philippe/Ulf.
ChangeLog v1-v2:
- We used only two states initially: default and sleep. It turns
out you can save some energy when idling (between transfers)
and even more when suspending on our platform, so grab all
three states and use them as applicable.
---
drivers/i2c/busses/i2c-nomadik.c | 89
1 file changed, 89 insertions(+)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 8b2ffcf..8d330dc 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -26,6 +26,7 @@
#include linux/platform_data/i2c-nomadik.h
#include linux/of.h
#include linux/of_i2c.h
+#include linux/pinctrl/consumer.h
#define DRIVER_NAME nmk-i2c
@@ -147,6 +148,10 @@ struct i2c_nmk_client {
* @stop: stop condition.
* @xfer_complete: acknowledge completion for a I2C message.
* @result: controller propogated result.
+ * @pinctrl: pinctrl handle.
+ * @pins_default: default state for the pins.
+ * @pins_idle: idle state for the pins.
+ * @pins_sleep: sleep state for the pins.
* @busy: Busy doing transfer.
*/
struct nmk_i2c_dev {
@@ -160,6 +165,11 @@ struct nmk_i2c_dev {
int stop;
struct completion xfer_complete;
int result;
+ /* Three pin states - default, idle sleep */
+ struct pinctrl *pinctrl;
+ struct pinctrl_state*pins_default;
+ struct pinctrl_state*pins_idle;
+ struct pinctrl_state*pins_sleep;
boolbusy;
};
@@ -636,6 +646,15 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
goto out_clk;
}
+ /* Optionaly enable pins to be muxed in and configured */
+ if (!IS_ERR(dev-pins_default)) {
+ status = pinctrl_select_state(dev-pinctrl,
+ dev-pins_default);
+ if (status)
+ dev_err(dev-adev-dev,
+ could not set default pins\n);
+ }
+
status = init_hw(dev);
if (status)
goto out;
@@ -663,6 +682,15 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
out:
clk_disable_unprepare(dev-clk);
out_clk:
+ /* Optionally let pins go into idle state */
+ if (!IS_ERR(dev-pins_idle)) {
+ status = pinctrl_select_state(dev-pinctrl,
+ dev-pins_idle);
+ if (status)
+ dev_err(dev-adev-dev,
+ could not set pins to idle state\n);
+ }
+
pm_runtime_put_sync(dev-adev-dev);
dev-busy = false;
@@ -857,15 +885,41 @@ static int nmk_i2c_suspend(struct device *dev)
{
struct amba_device *adev = to_amba_device(dev);
struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ int ret;
if (nmk_i2c-busy)
return -EBUSY;
+ if (!IS_ERR(nmk_i2c-pins_sleep)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_sleep);
+ if (ret)
+ dev_err(dev, could not set pins to sleep state\n);
+ }
+
return 0;
}
static int nmk_i2c_resume(struct device *dev)
{
+ struct amba_device *adev = to_amba_device(dev);
+ struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ int ret;
+
+ /* First go to the default state */
+ if (!IS_ERR(nmk_i2c-pins_default)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_default);
+ if (ret)
+ dev_err(dev, could not set pins to default state\n);
+ }
+ /* Then let's idle the pins until the next transfer happens */
+ if (!IS_ERR(nmk_i2c-pins_idle)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_idle);
+ if (ret)
+ dev_err(dev, could not set pins to idle state\n);
+ }
return 0;
}
#else
@@ -953,6 +1007,40 @@ static int
Re: [PATCH 1/2] misc/at24: Add at24c512b eeprom support
On Wed, Jan 23, 2013 at 1:50 PM, Wolfram Sang w.s...@pengutronix.de wrote: On Wed, Jan 23, 2013 at 01:24:52PM +0100, Linus Walleij wrote: On Wed, Jan 23, 2013 at 7:32 AM, Liu Ying ying@freescale.com wrote: This patch adds at24c512b eeprom support. The datasheet of at24c512b can be found at: http://www.alldatasheet.com/datasheet-pdf/pdf/ 256958/ATMEL/AT24C512B-TH-B.html Signed-off-by: Liu Ying ying@freescale.com Arnd Bergmann is the misc maintainer, route this by him. I usually take at24 patches via my I2C tree. Oh I didn't mean he'd merge it, I meant route it by him as in let him have a look at it :-) 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 1/2] misc/at24: Add at24c512b eeprom support
On Wed, Jan 23, 2013 at 7:32 AM, Liu Ying ying@freescale.com wrote: This patch adds at24c512b eeprom support. The datasheet of at24c512b can be found at: http://www.alldatasheet.com/datasheet-pdf/pdf/ 256958/ATMEL/AT24C512B-TH-B.html Signed-off-by: Liu Ying ying@freescale.com Arnd Bergmann is the misc maintainer, route this by him. 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
[PATCH] i2c: nomadik: adopt pinctrl support
From: Patrice Chotard patrice.chot...@stericsson.com
Amend the I2C nomadik pin controller to optionally take a pin control
handle and set the state of the pins to:
- default on boot, resume and before performing an i2c transfer
- idle after initial default, after resume default, and after each
i2c xfer
- sleep on suspend()
This should make it possible to optimize energy usage for the pins
both for the suspend/resume cycle, and for runtime cases inbetween
I2C transfers.
Signed-off-by: Patrice Chotard patrice.chot...@stericsson.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
ChangeLog v3-v4:
- Rebase onto v3.8-rc2
ChangeLog v2-v3:
- Rebase on top of the patch from Philippe/Ulf.
ChangeLog v1-v2:
- We used only two states initially: default and sleep. It turns
out you can save some energy when idling (between transfers)
and even more when suspending on our platform, so grab all
three states and use them as applicable.
---
drivers/i2c/busses/i2c-nomadik.c | 92
1 file changed, 92 insertions(+)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 8b2ffcf..8a5168a 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -26,6 +26,7 @@
#include linux/platform_data/i2c-nomadik.h
#include linux/of.h
#include linux/of_i2c.h
+#include linux/pinctrl/consumer.h
#define DRIVER_NAME nmk-i2c
@@ -147,6 +148,10 @@ struct i2c_nmk_client {
* @stop: stop condition.
* @xfer_complete: acknowledge completion for a I2C message.
* @result: controller propogated result.
+ * @pinctrl: pinctrl handle.
+ * @pins_default: default state for the pins.
+ * @pins_idle: idle state for the pins.
+ * @pins_sleep: sleep state for the pins.
* @busy: Busy doing transfer.
*/
struct nmk_i2c_dev {
@@ -160,6 +165,11 @@ struct nmk_i2c_dev {
int stop;
struct completion xfer_complete;
int result;
+ /* Three pin states - default, idle sleep */
+ struct pinctrl *pinctrl;
+ struct pinctrl_state*pins_default;
+ struct pinctrl_state*pins_idle;
+ struct pinctrl_state*pins_sleep;
boolbusy;
};
@@ -636,6 +646,15 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
goto out_clk;
}
+ /* Optionaly enable pins to be muxed in and configured */
+ if (!IS_ERR(dev-pins_default)) {
+ status = pinctrl_select_state(dev-pinctrl,
+ dev-pins_default);
+ if (status)
+ dev_err(dev-adev-dev,
+ could not set default pins\n);
+ }
+
status = init_hw(dev);
if (status)
goto out;
@@ -663,6 +682,15 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
out:
clk_disable_unprepare(dev-clk);
out_clk:
+ /* Optionally let pins go into idle state */
+ if (!IS_ERR(dev-pins_idle)) {
+ status = pinctrl_select_state(dev-pinctrl,
+ dev-pins_idle);
+ if (status)
+ dev_err(dev-adev-dev,
+ could not set pins to idle state\n);
+ }
+
pm_runtime_put_sync(dev-adev-dev);
dev-busy = false;
@@ -857,15 +885,44 @@ static int nmk_i2c_suspend(struct device *dev)
{
struct amba_device *adev = to_amba_device(dev);
struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ int ret;
if (nmk_i2c-busy)
return -EBUSY;
+ if (!IS_ERR(nmk_i2c-pins_sleep)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_sleep);
+ if (ret)
+ dev_err(dev,
+ could not set pins to sleep state\n);
+ }
+
return 0;
}
static int nmk_i2c_resume(struct device *dev)
{
+ struct amba_device *adev = to_amba_device(dev);
+ struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ int ret;
+
+ /* First go to the default state */
+ if (!IS_ERR(nmk_i2c-pins_default)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_default);
+ if (ret)
+ dev_err(dev,
+ could not set pins to default state\n);
+ }
+ /* Then let's idle the pins until the next transfer happens */
+ if (!IS_ERR(nmk_i2c-pins_idle)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_idle);
+ if (ret)
+ dev_err(dev,
+ could not set pins to idle state\n);
+ }
return 0;
}
#else
@@ -953,6 +1010,40
Re: [PATCH v2] i2c: nomadik: adopt pinctrl support
On Sat, Oct 6, 2012 at 3:30 PM, Wolfram Sang w.s...@pengutronix.de wrote: err_add_adap: + clk_unprepare(dev-clk); + err_prep_clk: This is unrelated, right? And it is also unneeded because of Ulf Hansson's patch? True. I'll roll out a v3 based on the latest i2c code. 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
[PATCH 2/2 v3] i2c: nomadik: adopt pinctrl support
From: Patrice Chotard patrice.chot...@stericsson.com
Amend the I2C nomadik pin controller to optionally take a pin control
handle and set the state of the pins to:
- default on boot, resume and before performing an i2c transfer
- idle after initial default, after resume default, and after each
i2c xfer
- sleep on suspend()
This should make it possible to optimize energy usage for the pins
both for the suspend/resume cycle, and for runtime cases inbetween
I2C transfers.
Signed-off-by: Patrice Chotard patrice.chot...@stericsson.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
ChangeLog v2-v3:
- Rebase on top of the patch from Philippe/Ulf.
ChangeLog v1-v2:
- We used only two states initially: default and sleep. It turns
out you can save some energy when idling (between transfers)
and even more when suspending on our platform, so grab all
three states and use them as applicable.
---
drivers/i2c/busses/i2c-nomadik.c | 101 +++
1 file changed, 101 insertions(+)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 3eeae52..d50b16a 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -24,6 +24,7 @@
#include linux/io.h
#include linux/pm_runtime.h
#include linux/platform_data/i2c-nomadik.h
+#include linux/pinctrl/consumer.h
#define DRIVER_NAME nmk-i2c
@@ -145,6 +146,10 @@ struct i2c_nmk_client {
* @stop: stop condition.
* @xfer_complete: acknowledge completion for a I2C message.
* @result: controller propogated result.
+ * @pinctrl: pinctrl handle.
+ * @pins_default: default state for the pins.
+ * @pins_idle: idle state for the pins.
+ * @pins_sleep: sleep state for the pins.
* @busy: Busy doing transfer.
*/
struct nmk_i2c_dev {
@@ -158,6 +163,11 @@ struct nmk_i2c_dev {
int stop;
struct completion xfer_complete;
int result;
+ /* Three pin states - default, idle sleep */
+ struct pinctrl *pinctrl;
+ struct pinctrl_state*pins_default;
+ struct pinctrl_state*pins_idle;
+ struct pinctrl_state*pins_sleep;
boolbusy;
};
@@ -648,6 +658,15 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
goto out_clk;
}
+ /* Optionaly enable pins to be muxed in and configured */
+ if (!IS_ERR(dev-pins_default)) {
+ status = pinctrl_select_state(dev-pinctrl,
+ dev-pins_default);
+ if (status)
+ dev_err(dev-adev-dev,
+ could not set default pins\n);
+ }
+
status = init_hw(dev);
if (status)
goto out;
@@ -675,6 +694,15 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
out:
clk_disable_unprepare(dev-clk);
out_clk:
+ /* Optionally let pins go into idle state */
+ if (!IS_ERR(dev-pins_idle)) {
+ status = pinctrl_select_state(dev-pinctrl,
+ dev-pins_idle);
+ if (status)
+ dev_err(dev-adev-dev,
+ could not set pins to idle state\n);
+ }
+
pm_runtime_put_sync(dev-adev-dev);
dev-busy = false;
@@ -869,15 +897,44 @@ static int nmk_i2c_suspend(struct device *dev)
{
struct amba_device *adev = to_amba_device(dev);
struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ int ret;
if (nmk_i2c-busy)
return -EBUSY;
+ if (!IS_ERR(nmk_i2c-pins_sleep)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_sleep);
+ if (ret)
+ dev_err(dev,
+ could not set pins to sleep state\n);
+ }
+
return 0;
}
static int nmk_i2c_resume(struct device *dev)
{
+ struct amba_device *adev = to_amba_device(dev);
+ struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ int ret;
+
+ /* First go to the default state */
+ if (!IS_ERR(nmk_i2c-pins_default)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_default);
+ if (ret)
+ dev_err(dev,
+ could not set pins to default state\n);
+ }
+ /* Then let's idle the pins until the next transfer happens */
+ if (!IS_ERR(nmk_i2c-pins_idle)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_idle);
+ if (ret)
+ dev_err(dev,
+ could not set pins to idle state\n);
+ }
return 0;
}
#else
@@ -941,6 +998,40 @@ static int nmk_i2c_probe(struct
[PATCH v2] i2c: nomadik: adopt pinctrl support
From: Patrice Chotard patrice.chot...@stericsson.com
Amend the I2C nomadik pin controller to optionally take a pin control
handle and set the state of the pins to:
- default on boot, resume and before performing an i2c transfer
- idle after initial default, after resume default, and after each
i2c xfer
- sleep on suspend()
This should make it possible to optimize energy usage for the pins
both for the suspend/resume cycle, and for runtime cases inbetween
I2C transfers.
Signed-off-by: Patrice Chotard patrice.chot...@stericsson.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
ChangeLog v1-v2:
- We used only two states initially: default and sleep. It turns
out you can save some energy when idling (between transfers)
and even more when suspending on our platform, so grab all
three states and use them as applicable.
---
drivers/i2c/busses/i2c-nomadik.c | 102 +++
1 file changed, 102 insertions(+)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 1b898b6..bd3da46 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -24,6 +24,7 @@
#include linux/io.h
#include linux/pm_runtime.h
#include linux/platform_data/i2c-nomadik.h
+#include linux/pinctrl/consumer.h
#define DRIVER_NAME nmk-i2c
@@ -145,6 +146,10 @@ struct i2c_nmk_client {
* @stop: stop condition.
* @xfer_complete: acknowledge completion for a I2C message.
* @result: controller propogated result.
+ * @pinctrl: pinctrl handle.
+ * @pins_default: default state for the pins.
+ * @pins_idle: idle state for the pins.
+ * @pins_sleep: sleep state for the pins.
* @busy: Busy doing transfer.
*/
struct nmk_i2c_dev {
@@ -158,6 +163,11 @@ struct nmk_i2c_dev {
int stop;
struct completion xfer_complete;
int result;
+ /* Three pin states - default, idle sleep */
+ struct pinctrl *pinctrl;
+ struct pinctrl_state*pins_default;
+ struct pinctrl_state*pins_idle;
+ struct pinctrl_state*pins_sleep;
boolbusy;
};
@@ -642,6 +652,15 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
pm_runtime_get_sync(dev-adev-dev);
+ /* Optionaly enable pins to be muxed in and configured */
+ if (!IS_ERR(dev-pins_default)) {
+ status = pinctrl_select_state(dev-pinctrl,
+ dev-pins_default);
+ if (status)
+ dev_err(dev-adev-dev,
+ could not set default pins\n);
+ }
+
clk_enable(dev-clk);
status = init_hw(dev);
@@ -670,6 +689,16 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
out:
clk_disable(dev-clk);
+
+ /* Optionally let pins go into idle state */
+ if (!IS_ERR(dev-pins_idle)) {
+ status = pinctrl_select_state(dev-pinctrl,
+ dev-pins_idle);
+ if (status)
+ dev_err(dev-adev-dev,
+ could not set pins to idle state\n);
+ }
+
pm_runtime_put_sync(dev-adev-dev);
dev-busy = false;
@@ -864,15 +893,44 @@ static int nmk_i2c_suspend(struct device *dev)
{
struct amba_device *adev = to_amba_device(dev);
struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ int ret;
if (nmk_i2c-busy)
return -EBUSY;
+ if (!IS_ERR(nmk_i2c-pins_sleep)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_sleep);
+ if (ret)
+ dev_err(dev,
+ could not set pins to sleep state\n);
+ }
+
return 0;
}
static int nmk_i2c_resume(struct device *dev)
{
+ struct amba_device *adev = to_amba_device(dev);
+ struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+ int ret;
+
+ /* First go to the default state */
+ if (!IS_ERR(nmk_i2c-pins_default)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_default);
+ if (ret)
+ dev_err(dev,
+ could not set pins to default state\n);
+ }
+ /* Then let's idle the pins until the next transfer happens */
+ if (!IS_ERR(nmk_i2c-pins_idle)) {
+ ret = pinctrl_select_state(nmk_i2c-pinctrl,
+ nmk_i2c-pins_idle);
+ if (ret)
+ dev_err(dev,
+ could not set pins to idle state\n);
+ }
return 0;
}
#else
@@ -936,6 +994,40 @@ static int nmk_i2c_probe(struct amba_device *adev, const
struct amba_id *id)
dev-adev = adev
