Re: [linux-sunxi] [PATCH] i2c: mv64xxx: The n clockdiv factor is 0 based on sunxi SoCs
On Sun, Sep 27, 2015 at 06:53:03PM +0200, Andrew Lunn wrote: > > >+ if (of_device_is_compatible(np, "allwinner,sun4i-a10-i2c") || > > >+ of_device_is_compatible(np, "allwinner,sun6i-a31-i2c")) > > Rather than have to extend this list every so often, how about adding > a helper of_device_is_compatible_vendor(), so you can just have: I don't know, I kind of like the fact that it's explicit. If we ever have another SoC coming in with a different behaviour, we won't have to expand it back. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: The n clockdiv factor is 0 based on sunxi SoCs
On Sun, Sep 27, 2015 at 04:57:08PM +0200, Hans de Goede wrote: > According to the datasheets to n factor for dividing the tclk is > 2 to the power n on Allwinner SoCs, not 2 to the power n + 1 as it is > on other mv64xxx implementations. > > I've contacted Allwinner about this and they have confirmed that the > datasheet is correct. > > This commit fixes the clk-divider calculations for Allwinner SoCs > accordingly. > > Signed-off-by: Hans de Goede Acked-by: Maxime Ripard Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] [PATCH] i2c: mv64xxx: The n clockdiv factor is 0 based on sunxi SoCs
On Sun, Sep 27, 2015 at 06:05:35PM +0200, Olliver Schinagl wrote: > Hey Hans, > > On 27-09-15 16:57, Hans de Goede wrote: > >According to the datasheets to n factor for dividing the tclk is > >2 to the power n on Allwinner SoCs, not 2 to the power n + 1 as it is > >on other mv64xxx implementations. > Ah! > > > >I've contacted Allwinner about this and they have confirmed that the > >datasheet is correct. > > > >This commit fixes the clk-divider calculations for Allwinner SoCs > >accordingly. > So this explains why all my i2c frequenties are double of what I setup. > Thanks for taking the time of figuring it out! I'll give it a test hopefully > soon. It would have been great to let us know... Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH RFC] eeprom: at24: extend driver to plug into the NVMEM framework
On Thu, Aug 20, 2015 at 06:38:51PM +0200, Andrew Lunn wrote: > > It's true that this is something that we might have overlooked. Is it > > expected to maintain that compatibility when moving a driver from one > > framework to another (and this is a real question, not a troll)? > > Yes. There will be user space applications reading from the eeprom > file in /sys. In fact, until the NVMEM framework arrived, it was not > easy to access the eeprom from kernel space, meaning the majority of > users must of been user space... Ack. > > If so, we might provide a compatibility layer to add the former file > > too, protected by a kconfig option maybe ? > > There is one other detail you might of missed. Both AT24 and AT25 do > have an in kernel API. In the at24_platform_data you can have a > callback function "setup" which gets called when the device is > probed. setup() is called with a struct memory_accessor which contains > function pointers for reading and writing to the EEPROM. A few > platforms use these for getting the MAC address out of the EEPROM. > And these platforms are old style, not DT. Actually, we took it into account. The in-kernel API is even a big chunk of the framework. The only thing we still need to figure out is what interface we need to register cells statically. AT25's memory accessor can be removed, there's no users for it. The only user of the AT24 is some omap l138 boards is mach-davinci. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH RFC] eeprom: at24: extend driver to plug into the NVMEM framework
On Mon, Aug 17, 2015 at 05:25:04PM +0200, Andrew Lunn wrote: > On Mon, Aug 17, 2015 at 03:59:23PM +0100, Srinivas Kandagatla wrote: > > > > > > On 17/08/15 14:09, Andrew Lunn wrote: > > >On Mon, Aug 17, 2015 at 02:01:24PM +0100, Srinivas Kandagatla wrote: > > >> > > >>+Adding Maxime in the loop > > >> > > >>On 16/08/15 16:37, Stefan Wahren wrote: > > >>>>>Another question which spring to mind is, do we want the eeprom to be > > >>>>>in /sys twice, the old and the new way? Backwards compatibility says > > >>>>>the old must stay. Do we want a way to suppress the new? Or should we > > >>>>>be going as far as refractoring the code into a core library, and two > > >>>>>wrapper drivers, old and new? > > >>>I think these are questions for the framework maintainers. > > >>> > > >>One of the reasons for the NVMEM framework is to remove that > > >>duplicate code in the every driver. There was no framework/ABI > > >>which was guiding such old eeprom sysfs entry in first place, so I > > >>dont see an issue in removing it for good. Correct me if am wrong. > > > > > >The reason for keeping it is backwards compatibility. Having the > > >contents of the EEPROM as a file in /sys via this driver is now a part > > >of the Linux ABI. You cannot argue it is not an ABI, just because > > >there is no framework. Userspace will be assuming it exists at the > > >specified location. So we cannot remove it, for existing uses of the > > >driver. > > Am Ok as long as someone is happy to maintain it. > > Wolfram Sang has been maintaining the AT24 driver since 2008. We need > his ACK to this change, and since this is an i2c driver, he is also > probably the path into mainline for this change. > > But we should also look at the bigger picture. The AT25, MAX6875 and > sunxi_sid drivers also have a binary file in /sys. It would be good to > have some sort of plan what to do with these drivers, even if at the > moment only AT24 is under discussion. It's true that this is something that we might have overlooked. Is it expected to maintain that compatibility when moving a driver from one framework to another (and this is a real question, not a troll)? If so, we might provide a compatibility layer to add the former file too, protected by a kconfig option maybe ? In the sunxi_sid case, I'm not sure anyone will really notice. It wasn't used for anything but debug at this point, but it will be noticed for the much more generic AT24 and At25 drivers. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 1/4] i2c: sunxi: Add Reduced Serial Bus (RSB) support
On Thu, Mar 05, 2015 at 07:40:44PM +0100, Wolfram Sang wrote: > > > > I don't have the bandwidth for a full review right now. However, I > > > already wanted to tell you guys that my gut feeling is that this > > > protocol is quite far away from I2C. P2WI was already at the edge. > > > Maybe there is a better place for such custom stuff? I dunno yet. > > > > That's unfortunate, especially since it looks closer to SPI than what > > P2WI even was. > > SPI? I assume you mean I2C. Can you elaborate your reasoning? Yeah, I obviously meant I2C, sorry. P2WI had no address. It was a single-device bus. However, the way it communicated with the device was very close to I2C, apart from a parity bit instead of the ACK. From that regard, RSB is a multiple device bus, using addresses, just like I2C. The way it communicates is basically the one used by P2WI. So really, it just is more I2C-alike than P2WI has ever been. > > What would be your suggestion? > > Let me quote: > > "I don't have the bandwidth for a full review right now... I dunno > yet." Good thing that we are not talking about a full review then, but more a philosophical discussion. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 1/4] i2c: sunxi: Add Reduced Serial Bus (RSB) support
Hi Wolfram,
On Wed, Mar 04, 2015 at 06:27:11PM +0100, Wolfram Sang wrote:
> On Mon, Mar 02, 2015 at 04:24:43PM +0800, Chen-Yu Tsai wrote:
> > The RSB controller looks like an SMBus controller which only supports byte
> > and word data transfers. It can also do double-word data transfers, but the
> > I2C subsystem does not support this, nor have we seen devices using this.
> >
> > The RSB differs from standard SMBus protocol on several aspects:
> > - it uses addresses set at runtime to address slaves. Runtime addresses
> > are sent to slaves using their 12bit hardware addresses. Up to 15
> > runtime addresses are available.
> > - it adds a parity bit every 8bits of data and address for read and
> > write accesses; this replaces the ack bit
> > - only one read access is required to read a byte (instead of a write
> > followed by a read access in standard SMBus protocol)
> > - there's no Ack bit after each read access
> >
> > This means this bus cannot be used to interface with standard SMBus
> > devices (known devices supporting this interface are the AXP223, AXP806,
> > AXP809 PMICs and the AC100 codec/RTC). However the RSB protocol is an
> > extension of P2WI, which was close enough to SMBus to be integrated into
> > the I2C subsystem in commit 3e833490fae5 ("i2c: sunxi: add P2WI (Push/Pull
> > 2 Wire Interface) controller support").
> >
> > Signed-off-by: Chen-Yu Tsai
>
> I don't have the bandwidth for a full review right now. However, I
> already wanted to tell you guys that my gut feeling is that this
> protocol is quite far away from I2C. P2WI was already at the edge.
> Maybe there is a better place for such custom stuff? I dunno yet.
That's unfortunate, especially since it looks closer to SPI than what
P2WI even was.
What would be your suggestion?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v2 1/4] i2c: sunxi: Add Reduced Serial Bus (RSB) support
of_node_full_name(child));
> + continue;
> + }
> +
> + /* get runtime address */
> + ret = of_property_read_u32(child, "reg", &rt_addr);
> + if (ret) {
> + dev_warn(dev, "runtime address not given for %s\n",
> + of_node_full_name(child));
> + continue;
> + }
Isn't that stored in the struct i2c_client address field already?
> + /* check runtime address */
> + ret = rsb_check_rt_addr(rt_addr);
> + if (ret) {
> + dev_warn(dev, "runtime address for %s is invalid\n",
> + of_node_full_name(child));
> + continue;
> + }
> +
> + /* setup command parameters */
> + writel(RSB_CMD_STRA, rsb->regs + RSB_CMD);
> + writel(RSB_DAR_RTA(rt_addr) | RSB_DAR_DA(hw_addr),
> +rsb->regs + RSB_DAR);
> +
> + /* send command */
> + ret = rsb_run_xfer(rsb);
> + if (ret)
> + dev_warn(dev, "set runtime address failed for %s\n",
> + of_node_full_name(child));
> + }
> +
> + return 0;
> +}
> +
> +
> +static const struct of_device_id rsb_of_match_table[] = {
> + { .compatible = "allwinner,sun8i-a23-rsb" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, rsb_of_match_table);
> +
> +static int rsb_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + unsigned long parent_clk_freq;
> + u32 clk_freq = 10;
> + struct resource *r;
> + struct rsb *rsb;
> + int clk_div;
> + int irq;
> + int ret;
> +
> + of_property_read_u32(np, "clock-frequency", &clk_freq);
> + if (clk_freq > RSB_MAX_FREQ) {
> + dev_err(dev,
> + "clock-frequency (%u Hz) is too high (max = 20MHz)",
> + clk_freq);
> + return -EINVAL;
> + }
> +
> + rsb = devm_kzalloc(dev, sizeof(struct rsb), GFP_KERNEL);
> + if (!rsb)
> + return -ENOMEM;
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + rsb->regs = devm_ioremap_resource(dev, r);
> + if (IS_ERR(rsb->regs))
> + return PTR_ERR(rsb->regs);
> +
> + strlcpy(rsb->adapter.name, pdev->name, sizeof(rsb->adapter.name));
Isn't that what strcpy is doing already?
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "failed to retrieve irq: %d\n", irq);
> + return irq;
> + }
> +
> + rsb->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(rsb->clk)) {
> + ret = PTR_ERR(rsb->clk);
> + dev_err(dev, "failed to retrieve clk: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(rsb->clk);
> + if (ret) {
> + dev_err(dev, "failed to enable clk: %d\n", ret);
> + return ret;
> + }
> +
> + parent_clk_freq = clk_get_rate(rsb->clk);
> +
> + rsb->rstc = devm_reset_control_get(dev, NULL);
> + if (IS_ERR(rsb->rstc)) {
> + ret = PTR_ERR(rsb->rstc);
> + dev_err(dev, "failed to retrieve reset controller: %d\n", ret);
> + goto err_clk_disable;
> + }
> +
> + ret = reset_control_deassert(rsb->rstc);
> + if (ret) {
> + dev_err(dev, "failed to deassert reset line: %d\n", ret);
> + goto err_clk_disable;
> + }
> +
> + init_completion(&rsb->complete);
> + strlcpy(rsb->adapter.name, RSB_CTRL_NAME, sizeof(rsb->adapter.name));
> + rsb->adapter.dev.parent = dev;
> + rsb->adapter.algo = &rsb_algo;
> + rsb->adapter.owner = THIS_MODULE;
Is the owner field still needed?
> + rsb->adapter.dev.of_node = pdev->dev.of_node;
> + platform_set_drvdata(pdev, rsb);
> + i2c_set_adapdata(&rsb->adapter, rsb);
> +
> + writel(RSB_CTRL_SOFT_RST, rsb->regs + RSB_CTRL);
> +
> + clk_div = parent_clk_freq / clk_freq;
> + if (!clk_div) {
> + dev_warn(dev,
> + "clock-frequency is too high, setting it to %lu Hz\n",
> + parent_clk_freq);
> + clk_div = 1;
> + } else if (clk_div > RSB_CCR_MAX_CLK_DIV) {
> + dev_warn(dev,
> + "clock-frequency is too low, setting it to %lu Hz\n",
> + parent_clk_freq / RSB_CCR_MAX_CLK_DIV);
> + clk_div = RSB_CCR_MAX_CLK_DIV;
> + }
Why not just error out?
> + writel(RSB_CCR_SDA_OUT_DELAY(1) | RSB_CCR_CLK_DIV(clk_div),
> +rsb->regs + RSB_CCR);
> +
> + ret = devm_request_irq(dev, irq, rsb_interrupt, 0, RSB_CTRL_NAME, rsb);
> + if (ret) {
> + dev_err(dev, "can't register interrupt handler irq%d: %d\n",
> + irq, ret);
> + goto err_reset_assert;
> + }
> +
> + rsb_init_device_mode(rsb);
> +
> + ret = rsb_set_rt_addrs(rsb);
> + if (ret)
> + goto err_reset_assert;
> +
> + ret = i2c_add_adapter(&rsb->adapter);
> + if (!ret)
> + return 0;
> +
> +err_reset_assert:
> + reset_control_assert(rsb->rstc);
> +
> +err_clk_disable:
> + clk_disable_unprepare(rsb->clk);
> +
> + return ret;
> +}
> +
> +static int rsb_remove(struct platform_device *dev)
> +{
> + struct rsb *rsb = platform_get_drvdata(dev);
> +
> + i2c_del_adapter(&rsb->adapter);
> + reset_control_assert(rsb->rstc);
> + clk_disable_unprepare(rsb->clk);
> +
> + return 0;
> +}
> +
> +static struct platform_driver rsb_driver = {
> + .probe = rsb_probe,
> + .remove = rsb_remove,
> + .driver = {
> + .name = "i2c-sunxi-rsb",
> + .of_match_table = rsb_of_match_table,
> + },
> +};
> +module_platform_driver(rsb_driver);
> +
> +MODULE_AUTHOR("Chen-Yu Tsai ");
> +MODULE_DESCRIPTION("Allwinner RSB driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.1.4
>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v2 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation
On Mon, Mar 02, 2015 at 04:24:44PM +0800, Chen-Yu Tsai wrote: > Reduced Serial Bus (RSB) is an SMBus like bus used to communicate > with some PMICs (like the AXP223) or other peripherals. > > The RSB DT bindings are pretty much the same as the one defined for > the marvell's mv64xxx controller, with the additional RSB specific > "allwinner,rsb-hw-addr" property for slave device nodes. > > There are 2 types of addresses for RSB devices, a hardware address > and a runtime (software) configurable address. The former is only > used when configuring the latter. All read/write accesses use the > runtime address. > > It would seem straightforward to use the hardware address in the > DT bindings as the slave's address. However this will not work as > the hardware address is 12 bits wide, and at least 1 device, the > AC100 audio codec, has the highest bit set. This address would be > incompatible with I2C (7 or 10 bit addresses) and likely rejected. > > Hence this binding uses statically allocated (by the author of the > DT) runtime addresses for the slave's "reg" property. The hardware > address is put in a separete named property. When writing a new DT, ^ separate > the author must take care to not have multiple slave devices use > the same address. It is recommended to follow whatever conventions > the hardware vendor uses. While very complete, the three last paragraphs should rather be, or at least duplicated, in the file itself. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH V2] i2c-designware: Add Intel Baytrail PMIC I2C bus support
Hi David, On Tue, Oct 07, 2014 at 12:14:20PM -0700, David E. Box wrote: > Hi Maxime, > > On Thu, Sep 25, 2014 at 11:47:52AM +0200, Maxime Ripard wrote: > > Hi David, > > > > On Tue, Sep 23, 2014 at 12:58:54PM -0700, David E. Box wrote: > > > Hi Maxime, > > > > > > On Tue, Sep 23, 2014 at 09:00:57PM +0200, Maxime Ripard wrote: > > > > Hi David, > > > > > > > > On Tue, Sep 23, 2014 at 11:40:26AM -0700, David E. Box wrote: > > > > > This patch implements an I2C bus sharing mechanism between the host > > > > > and platform > > > > > hardware on select Intel BayTrail SoC platforms using the X-Powers > > > > > AXP288 PMIC. > > > > > > > > > > On these platforms access to the PMIC must be shared with platform > > > > > hardware. The > > > > > hardware unit assumes full control of the I2C bus and the host must > > > > > request > > > > > access through a special semaphore. Hardware control of the bus also > > > > > makes it > > > > > necessary to disable runtime pm to avoid interfering with hardware > > > > > transactions. > > > > > > > > > > Signed-off-by: David E. Box > > > > > > > > Sorry for stepping in like this without really knowing your platform, > > > > but wouldn't using the hwspinlock framework make more sense than > > > > hardcoding your own internal functions here? > > > > > > I looked into this but didn't see a clear way on our platform to identify > > > the > > > semaphore seperately from doing it in the designware platform driver. The > > > way > > > we can find it now is through evaluating an ACPI _SEM object on every i2c > > > device > > > that gets probed by the dw driver since at probe time we can get the acpi > > > handle. > > > > And you have no way to turn it around and identify which semaphore is > > associated to which i2c bus? > > > > If so, there is probably some way to associate a given instance of the > > i2c driver to one semaphore. > > > > > Without this handle however there isn't a clear way of evaluating the _SEM > > > object which would be needed to register a hwspinlock in separate code. > > > > > > Plus it would still require changes to the designware i2c core, though > > > admittedly > > > having a generic hwspinlock pointer added to the struct is cleaner. > > > > Not only cleaner, but that could also be used by other platforms that > > are using this I2C driver (and since it's a designware IP, there must > > be quite a lot) together with hardware locking. > > > > After again considering a way to make this work I don't think this api can fit > well with our platform. Acquisition of this semaphore is through a mailbox > sequence where we set one register and then poll another for a value that > confirms we have the lock. For best performance we need to be able to > periodically sleep while waiting for that confirmation. This time can vary > widely as it's dependent on the component we are requesting the semaphore from > which is itself a user of that bus. > > While we could simply fail after a short time, reattempts would still need > to happen in the i2c-designware driver and the timing would be completely > dependent on our hardware, making it less clean for reuse. In addition, > if we timed out, we would have to immediately call unlock to cancel the > mailbox transaction. This may not fit well with reuse either. Ok, if Wolfram is ok with it, and if it makes your life much easier, I'm ok :) Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH V2] i2c-designware: Add Intel Baytrail PMIC I2C bus support
Hi David, On Tue, Sep 23, 2014 at 12:58:54PM -0700, David E. Box wrote: > Hi Maxime, > > On Tue, Sep 23, 2014 at 09:00:57PM +0200, Maxime Ripard wrote: > > Hi David, > > > > On Tue, Sep 23, 2014 at 11:40:26AM -0700, David E. Box wrote: > > > This patch implements an I2C bus sharing mechanism between the host and > > > platform > > > hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 > > > PMIC. > > > > > > On these platforms access to the PMIC must be shared with platform > > > hardware. The > > > hardware unit assumes full control of the I2C bus and the host must > > > request > > > access through a special semaphore. Hardware control of the bus also > > > makes it > > > necessary to disable runtime pm to avoid interfering with hardware > > > transactions. > > > > > > Signed-off-by: David E. Box > > > > Sorry for stepping in like this without really knowing your platform, > > but wouldn't using the hwspinlock framework make more sense than > > hardcoding your own internal functions here? > > I looked into this but didn't see a clear way on our platform to identify the > semaphore seperately from doing it in the designware platform driver. The way > we can find it now is through evaluating an ACPI _SEM object on every i2c > device > that gets probed by the dw driver since at probe time we can get the acpi > handle. And you have no way to turn it around and identify which semaphore is associated to which i2c bus? If so, there is probably some way to associate a given instance of the i2c driver to one semaphore. > Without this handle however there isn't a clear way of evaluating the _SEM > object which would be needed to register a hwspinlock in separate code. > > Plus it would still require changes to the designware i2c core, though > admittedly > having a generic hwspinlock pointer added to the struct is cleaner. Not only cleaner, but that could also be used by other platforms that are using this I2C driver (and since it's a designware IP, there must be quite a lot) together with hardware locking. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH V2] i2c-designware: Add Intel Baytrail PMIC I2C bus support
Hi David, On Tue, Sep 23, 2014 at 11:40:26AM -0700, David E. Box wrote: > This patch implements an I2C bus sharing mechanism between the host and > platform > hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 > PMIC. > > On these platforms access to the PMIC must be shared with platform hardware. > The > hardware unit assumes full control of the I2C bus and the host must request > access through a special semaphore. Hardware control of the bus also makes it > necessary to disable runtime pm to avoid interfering with hardware > transactions. > > Signed-off-by: David E. Box Sorry for stepping in like this without really knowing your platform, but wouldn't using the hwspinlock framework make more sense than hardcoding your own internal functions here? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: continue probe when clock-frequency is missing
On Mon, Aug 25, 2014 at 11:50:19PM +0800, Chen-Yu Tsai wrote: > The "clock-frequency" DT property is listed as optional, However, > the current code stores the return value of of_property_read_u32 in > the return code of mv64xxx_of_config, but then forgets to clear it > after setting the default value of "clock-frequency". It is then > passed out to the main probe function, resulting in a probe failure > when "clock-frequency" is missing. > > This patch checks and then throws away the result of > of_property_read_u32, instead of storing it and having to clear it > afterwards. > > This issue was discovered after the property was removed from all > sunxi DTs. > > Signed-off-by: Chen-Yu Tsai > Cc: sta...@vger.kernel.org Good catch! Acked-by: Maxime Ripard Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support
Hi Arnd,
On Tue, Jun 10, 2014 at 03:54:56PM +0200, Arnd Bergmann wrote:
> On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote:
> >
> > +config I2C_SUN6I_P2WI
> > + tristate "Allwinner sun6i internal P2WI controller"
> > + depends on ARCH_SUNXI
> > + help
> > + If you say yes to this option, support will be included for the
> > + P2WI (Push/Pull 2 Wire Interface) controller embedded in some
> > sunxi
> > + SOCs.
> > + The P2WI looks like an SMBus controller (which supports only byte
> > + accesses), except that it only supports one slave device.
> > + This interface is used to connect to specific PMIC devices (like
> > the
> > + AXP221).
> > +
>
> Sorry for the stupid question, but why is this an i2c driver if the
> hardware protocol is completely different?
It's not completely different. It deviates, but still looks very
similar to i2c, and to be precise, SMBus.
You'll have the full discussion that led to do this in i2c here:
http://www.spinics.net/lists/linux-i2c/msg15066.html
Also, one significant thing to take into account is that the
communication with a device starts as I2C, only to switch to this
protocol after some initialization sequence.
> I understand that a lot of devices can be driven using either spi or
> i2c, and we have two sets of {directories,maintainers,bus_types,...}
> for them. Your description sounds like this is a separate option
> that isn't any closer to i2c than it is to spi.
That's not true. It's *much* closer from I2C than it is from SPI.
> Would it perhaps be better to expose it only as a regmap rather than
> an i2c host?
That could be a solution, but is it a common practice to define a bus
adapter driver in a regmap driver?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v4 2/2] ARM: sunxi: dt: Convert to the new i2c compatibles
On Wed, May 21, 2014 at 09:54:15AM +0200, Wolfram Sang wrote: > On Mon, Mar 31, 2014 at 02:54:58PM +0200, Maxime Ripard wrote: > > Switch the device tree to the new compatibles introduced in the i2c drivers > > to have a common pattern accross all Allwinner SoCs. > > > > Signed-off-by: Maxime Ripard > > I usually don't take dts updates, but I guess it makes sense this time, > because we dropped the old binding? > Either way is fine for me. It would be better to have the two patches in the same tree, but I don't have any issue with merging it. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v4] i2c: add driver for Rockchip RK3xxx SoC I2C adapter
On Mon, May 19, 2014 at 11:32:55AM +0200, Max Schwarz wrote: > Driver for the native I2C adapter found in Rockchip RK3xxx SoCs. > > Configuration is only possible through devicetree. The driver is > interrupt driven and supports the I2C_M_IGNORE_NAK mangling bit. > > Signed-off-by: Max Schwarz Reviewed-by: Maxime Ripard -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3] i2c: add driver for Rockchip RK3xxx SoC I2C adapter
; +static int rk3x_i2c_setup(struct rk3x_i2c *i2c, struct i2c_msg *msgs, int
> num)
> +{
> + u32 addr = (msgs[0].addr & 0x7f) << 1;
> + int ret = 0;
> +
> + /*
> + * The I2C adapter can issue a small (len < 4) write packet before
> + * reading. This speeds up SMBus-style register reads.
> + * The MRXADDR/MRXRADDR hold the slave address and the slave register
> + * address in this case.
> + */
> +
> + if (num >= 2 && msgs[0].len < 4
> + && !(msgs[0].flags & I2C_M_RD)
> + && (msgs[1].flags & I2C_M_RD)) {
> + u32 reg_addr = 0;
> +
> + dev_dbg(i2c->dev, "Combined write/read from addr 0x%x\n",
> + addr >> 1);
> +
> + if (msgs[0].len == 0)
> + return -EINVAL;
> +
> + /* Fill MRXRADDR with the register address(es) */
> + reg_addr = msgs[0].buf[0];
> + reg_addr |= REG_MRXADDR_LOW;
> +
> + if (msgs[0].len >= 2) {
> + reg_addr |= msgs[0].buf[1] << 8;
> + reg_addr |= REG_MRXADDR_MID;
> +
> + if (msgs[0].len >= 3) {
> + reg_addr |= msgs[0].buf[2] << 16;
> + reg_addr |= REG_MRXADDR_HIGH;
> + }
> + }
> +
> + /* msgs[0] is handled by hw. */
> + i2c->msg = &msgs[1];
> +
> + i2c->mode = REG_CON_MOD_REGISTER_TX;
> +
> + i2c_writel(i2c, addr | REG_MRXADDR_LOW, REG_MRXADDR);
> + i2c_writel(i2c, reg_addr, REG_MRXRADDR);
> +
> + ret = 2;
> + } else {
> + /* We'll have to do it the boring way and process the msgs
> + * one-by-one. */
> +
> + if (msgs[0].flags & I2C_M_RD) {
> + addr |= 1; /* set read bit */
> +
> + /* We have to transmit the slave addr first. Use
> + * MOD_REGISTER_TX for that purpose. */
> + i2c->mode = REG_CON_MOD_REGISTER_TX;
> + i2c_writel(i2c, addr | REG_MRXADDR_LOW, REG_MRXADDR);
> + i2c_writel(i2c, 0, REG_MRXRADDR);
> + } else {
> + i2c->mode = REG_CON_MOD_TX;
> + }
> +
> + i2c->msg = &msgs[0];
> +
> + ret = 1;
> + }
> +
> + i2c->addr = msgs[0].addr;
> + i2c->busy = true;
> + i2c->state = STATE_START;
> + i2c->processed = 0;
> + i2c->error = 0;
> +
> + rk3x_i2c_clean_ipd(i2c);
> +
> + return ret;
> +}
> +
> +static int rk3x_i2c_xfer(struct i2c_adapter *adap,
> + struct i2c_msg *msgs, int num)
> +{
> + struct rk3x_i2c *i2c = (struct rk3x_i2c *)adap->algo_data;
> + unsigned long timeout, flags;
> + int ret = 0;
> + int i;
> +
> + spin_lock_irqsave(&i2c->lock, flags);
> +
> + clk_enable(i2c->clk);
> +
> + /* The clock rate might have changed, so setup the divider again */
> + rk3x_i2c_set_scl_rate(i2c, i2c->scl_frequency);
> +
> + /* Process msgs. We can handle more than one message at once (see
> + * rk3x_i2c_setup()). */
> + for (i = 0; i < num; i += ret) {
> + ret = rk3x_i2c_setup(i2c, msgs + i, num);
> +
> + if (ret < 0) {
> + dev_err(i2c->dev, "rk3x_i2c_setup() failed\n");
> + break;
> + }
> +
> + spin_unlock_irqrestore(&i2c->lock, flags);
> +
> + rk3x_i2c_start(i2c);
> +
> + timeout = wait_event_timeout(i2c->wait, !i2c->busy,
> + msecs_to_jiffies(WAIT_TIMEOUT));
> +
> + spin_lock_irqsave(&i2c->lock, flags);
> +
> + if (timeout == 0) {
> + dev_err(i2c->dev, "timeout, ipd: 0x%08X",
> + i2c_readl(i2c, REG_IPD));
> +
> + /* Force a STOP condition without interrupt */
> + i2c_writel(i2c, 0, REG_IEN);
> + i2c_writel(i2c, REG_CON_EN | REG_CON_STOP, REG_CON);
> +
> + i2c->state = STATE_IDLE;
> +
> + ret = -ETIMEDOUT;
> + break;
> + }
> +
> + if (i2c->error) {
> + ret = i2c->error;
> + break;
> + }
> + }
> +
> + clk_disable(i2c->clk);
> + spin_unlock_irqrestore(&i2c->lock, flags);
> +
> + return ret;
> +}
> +
> +static u32 rk3x_i2c_func(struct i2c_adapter *adap)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
> +}
> +
> +static const struct i2c_algorithm rk3x_i2c_algorithm = {
> + .master_xfer= rk3x_i2c_xfer,
> + .functionality = rk3x_i2c_func,
> +};
> +
> +static const struct of_device_id rk3x_i2c_match[];
> +
> +static int rk3x_i2c_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *match;
> + struct rk3x_i2c *i2c;
> + struct resource *mem;
> + int ret = 0;
> + u32 value;
> +
> + if (!pdev->dev.of_node) {
> + dev_err(&pdev->dev, "rk3x-i2c needs a devicetree node\n");
> + return -EINVAL;
> + }
> +
> + i2c = devm_kzalloc(&pdev->dev, sizeof(struct rk3x_i2c), GFP_KERNEL);
> + if (!i2c)
> + return -ENOMEM;
> +
> + match = of_match_node(rk3x_i2c_match, np);
> + i2c->soc_data = (struct rk3x_i2c_soc_data *)match->data;
> +
> + if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> + &i2c->scl_frequency)) {
> + i2c->scl_frequency = 100 * 1000;
> + dev_info(&pdev->dev, "using default SCL frequency: 100kHz\n");
> + }
> +
> + strlcpy(i2c->adap.name, "rk3x-i2c", sizeof(i2c->adap.name));
> + i2c->adap.owner = THIS_MODULE;
> + i2c->adap.algo = &rk3x_i2c_algorithm;
> + i2c->adap.retries = 3;
> + i2c->adap.dev.of_node = np;
> + i2c->adap.algo_data = i2c;
> + i2c->adap.dev.parent = &pdev->dev;
> +
> + i2c->dev = &pdev->dev;
> +
> + spin_lock_init(&i2c->lock);
> + init_waitqueue_head(&i2c->wait);
> +
> + i2c->clk = devm_clk_get(&pdev->dev, 0);
> + if (IS_ERR(i2c->clk)) {
> + dev_err(&pdev->dev, "cannot get clock\n");
> + return PTR_ERR(i2c->clk);
> + }
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + i2c->regs = devm_ioremap_resource(&pdev->dev, mem);
> + if (IS_ERR(i2c->regs))
> + return PTR_ERR(i2c->regs);
> +
> + /* Switch to new interface if the SoC also offers the old one.
> + * The control bit is located in the GRF register space. */
> + if (i2c->soc_data->grf_offset >= 0) {
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "rockchip,bus-index", &i2c->bus_index);
You probably can use the aliases id for this, instead of providing a
somewhat redundant information here?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v4 0/2] i2c: sunxi: Change compatibles pattern
Hi Wolfram, On Mon, Mar 31, 2014 at 02:54:56PM +0200, Maxime Ripard wrote: > Hi, > > This is the fourth version of the i2c compatible changes. > > Changes from v4: > - Added extra per-SoC compatibles to the DT for all SoC but the A10 Any chance you merge this for 3.16? I guess I addressed your concerns about the DT part. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v4 0/2] i2c: sunxi: Change compatibles pattern
Hi Wolfram, On Mon, Mar 31, 2014 at 02:54:56PM +0200, Maxime Ripard wrote: > Hi, > > This is the fourth version of the i2c compatible changes. > > Changes from v4: > - Added extra per-SoC compatibles to the DT for all SoC but the A10 I think I've addressed your comments in this version, are you ok with these changes? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 1/2] Create eeprom_dev hardware class for EEPROM devices
Hi, On Thu, Jan 23, 2014 at 11:16:01AM -0800, Curt Brune wrote: > Create a new hardware class under /sys/class/eeprom_dev > > EEPROM drivers can register their devices with the eeprom_dev class > during instantiation. > > The registered devices show up as: > > /sys/class/eeprom_dev/eeprom0 > /sys/class/eeprom_dev/eeprom1 > ... > /sys/class/eeprom_dev/eeprom[N] > > Each member of the eeprom class exports a sysfs file called "label", > containing the label property from the corresponding device tree node. > > Example: > > /sys/class/eeprom_dev/eeprom0/label > > If the device tree node property "label" does not exist the value > "unknown" is used. > > Note: The class cannot be called 'eeprom' as that is the name of the > I/O file created by the driver. The class name appears as a > sub-directory within the main device directory. Hence the class name > 'eeprom_dev'. > > Userspace can use the label to identify what the EEPROM is for. > > The real device is available from the class device via the "device" > link: > > /sys/class/eeprom_dev/eeprom0/device Sorry for stepping up this late, but I've been working on an eeprom framework recently to move all the duplication that exists in the eeprom drivers (such as sysfs file creation, in kernel accessors) to common code. And part of that work was to create a eeprom class. The only feature that's missing from your code is the label, but I guess it can easily be added, and makes sense. I'm not quite fond of some of the API yet, particularly on the in kernel side, but I guess I can post what I have so far as an RFC and get the discussion started. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: I2C adapters protocol deviation
On Sun, Apr 06, 2014 at 07:18:25PM +0200, Hans de Goede wrote: > Hi, > > On 04/06/2014 05:37 PM, Wolfram Sang wrote: > > On Sun, Apr 06, 2014 at 04:01:52PM +0200, Hans de Goede wrote: > >> Hi, > >> > >> On 04/04/2014 02:26 PM, Wolfram Sang wrote: > >>> > >>>> So what we really have is a single slave i2c host sort of. At least > >>>> we could model it like that. The host could be told which address to > >>>> listen to (and which single i2c write to do to init the pmic) through > >>>> devicetree and then all the differences would be hidden in the host > >>>> driver, ie we would check the slave-address and if it is not the single > >>>> one we support, we just return the appropriate error for a device not > >>>> acking, and everything should work as a regular i2c host which > >>>> only supports i2c_smbus_read_byte and i2c_smbus_write_byte. > >>> > >>> I'd think we need a new message flag like I2C_M_PUSHPULL which says that > >>> this message has only the direction bit instead of the address and needs > >>> a parity bit afterwards. In addition to that, we need a new > >>> functionality flag I2C_FUNC_PUSHPULL which means the host driver can > >>> handle those messages. So, the PMIC driver could query support for > >>> I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages > >>> using smbus functions with the new flag set. > >> > >> Thanks for the input this sounds good, I guess we'll give this a shot > >> when we get around to coding up support for the p2wi block in the A31. > > > > On a second thought, maybe more granularity is better. Like using > > I2C_M_DROP_ADDRESS and I2C_M_ADD_PARITY and then make > > I2C_CLIENT_PUSHPULL involve I2C_M_DROP_ADDRESS | I2C_M_ADD_PARITY. > > Hmm, I'm not completely sold on the whole idea of having special > flags, esp. since it seems that ie the AXP221 may operate in normal > i2c mode in some designs too. So ideally we would just hide from > clients that this is something else then plain i2c. So that we can have > an axp221 driver which is not even aware about this weird i2c-variant and > will just work independent on how the axp221 is hooked up. I don't think we actually saw in real life an AXP221 connected only using i2c. I'd say we shouldn't worry too much about a theorical corner case that we never saw, until we actually see it. > Likewise it would be useful to have the i2cdump utility just work, etc. I'm not sure I want the i2c-tools to start poking around the PMIC. > So maybe a flag which is a hint that this is special on the controller, > but I don't think we should be checking for special flags in the messages > on the controller side. Basically the whole p2wi allows reading / writing > byte registers, so I2C_FUNC_SMBUS_BYTE is a 1:1 mapping of the functionality, > as for the address, we can just check it is the one address used to do > the initial setup, and if it is not then just return an error. Yes, we obviously have to check for the address in the xfer function. > >>> Not sure about the I2C-to-PushPull switch: Is it 100% host configuration > >>> or does it also depend on the one slave attached? > >> > >> The datasheet we've suggests that it actually influences the one slave > >> attached. Note that u-boot on this machines will likely already have made > >> the switch, but I guess we don't want to count on that. > > > > Can we detect if this switching was already made? > > I don't think we can. But I think doing the switch a second time is ok / > does not result in an error. And it will probably mangle the PMIC configuration. I'm not very comfortable with that.. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: I2C adapters protocol deviation
On Sun, Apr 06, 2014 at 05:37:29PM +0200, Wolfram Sang wrote: > On Sun, Apr 06, 2014 at 04:01:52PM +0200, Hans de Goede wrote: > > Hi, > > > > On 04/04/2014 02:26 PM, Wolfram Sang wrote: > > > > > >> So what we really have is a single slave i2c host sort of. At least > > >> we could model it like that. The host could be told which address to > > >> listen to (and which single i2c write to do to init the pmic) through > > >> devicetree and then all the differences would be hidden in the host > > >> driver, ie we would check the slave-address and if it is not the single > > >> one we support, we just return the appropriate error for a device not > > >> acking, and everything should work as a regular i2c host which > > >> only supports i2c_smbus_read_byte and i2c_smbus_write_byte. > > > > > > I'd think we need a new message flag like I2C_M_PUSHPULL which says that > > > this message has only the direction bit instead of the address and needs > > > a parity bit afterwards. In addition to that, we need a new > > > functionality flag I2C_FUNC_PUSHPULL which means the host driver can > > > handle those messages. So, the PMIC driver could query support for > > > I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages > > > using smbus functions with the new flag set. > > > > Thanks for the input this sounds good, I guess we'll give this a shot > > when we get around to coding up support for the p2wi block in the A31. > > On a second thought, maybe more granularity is better. Like using > I2C_M_DROP_ADDRESS and I2C_M_ADD_PARITY and then make > I2C_CLIENT_PUSHPULL involve I2C_M_DROP_ADDRESS | I2C_M_ADD_PARITY. I'd agree with that. Other clients/adapters might need only one of these. Note that you'd probably need a I2C_M_DELAYED_ACK or something. > > > Not sure about the I2C-to-PushPull switch: Is it 100% host configuration > > > or does it also depend on the one slave attached? > > > > The datasheet we've suggests that it actually influences the one slave > > attached. Note that u-boot on this machines will likely already have made > > the switch, but I guess we don't want to count on that. > > Can we detect if this switching was already made? None that we're aware of. Since the PMIC is already most likely initialised, I think we can just use it as if it was already in P2WI mode. > > > Are there some datasheets available? > > > > The AXP221 is documented here: > > http://linux-sunxi.org/AXP221 > > This is translated by one of our community members from a Chinese datasheet. > > > > The P2WI interface is (somewhat) documented in the A31 datasheet, but we > > cannot > > share that in public. > > Any chance for me to get it if I sign something? Let me see what I can do. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: I2C adapters protocol deviation
On Thu, Apr 03, 2014 at 05:30:09PM +0200, Hans de Goede wrote: > Hi, > > On 04/03/2014 04:55 PM, Maxime Ripard wrote: > > Hi Wolfram, > > > > On the Allwinner A31, the PMIC communicates with the SoC through a bus > > looking quite similar to I2C, while being pretty different. > > > > The communication starts with the PMIC through the regular I2C > > protocol, but it's only used to initialize the PMIC, and switch to a > > mode called "Push/Pull 2 Wire Interface". > > > > That bus is using SDA and SCL, with the start and stop conditions > > exactly like I2C does, but: > > - Once the start condition has been issued, the address isn't sent, > > only a direction bit. Hence, it does not support multiple devices > > anymore. > > - Once that direction bit has been sent, the master sends the > > register it wants to read from/write to, over 8 bits, followed by > > one parity bit. > > - Whenever you're writing, the master then sends the data over 8 > > bits, followed once again by a parity bit. Then, and only then, an > > ACK is issued by the slave. > > - Whenever you're reading, the master then clocks SDL and the slave > > drives SDA for 8 bits plus 1 parity bit. If there was some kind of > > error, the slave will pull SDA up for 9 cycles, resulting in a > > parity error. Like with I2C though, since it is the only and last > > byte it's receiving, the master won't issue an ACK. > > > > Obviously, to go ahead with the PMIC support, we need to support this > > controller and bus first. I can't really decide whether it's within > > the scope of the allowed protocol deviations of I2C or if we should > > create a whole new bus for it. > > I've been thinking about this too. In the mean time I have slept quite a > few nights on this now and swung my opinion from use i2c subsys to do our > own bus and back again. But since I've actually added support for this > to u-boot and I now know the device better I believe using the i2c > subsys is the best solution for this. > > The PMIC does seem to start in i2c, and then gets switched to this new > push-pull 2 wire i2c-ish protocol on init. So it does have a slave address, > but that is used only once, and then the bus is for a single device only. > > Note there seems to be no way to use the host in a regular i2c mode. It has > a setup mode where it does a single regular i2c write (or so I believe) and > then it is in its own custom mode from there on. Well, in theory, you could. It has a register where you can control the levels of SDA and SCL, so you could bitbang the bus using them. But I'm not sure we want to do that :) > So what we really have is a single slave i2c host sort of. At least > we could model it like that. The host could be told which address to > listen to (and which single i2c write to do to init the pmic) through > devicetree and then all the differences would be hidden in the host > driver, ie we would check the slave-address and if it is not the single > one we support, we just return the appropriate error for a device not > acking, and everything should work as a regular i2c host which > only supports i2c_smbus_read_byte and i2c_smbus_write_byte. > > IOW the i2c_algorithm struct for the host would not set master_xfer, > only smbus_xfer and its functionality function would return > I2C_FUNC_SMBUS_BYTE Yep, it would make sense. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
I2C adapters protocol deviation
Hi Wolfram, On the Allwinner A31, the PMIC communicates with the SoC through a bus looking quite similar to I2C, while being pretty different. The communication starts with the PMIC through the regular I2C protocol, but it's only used to initialize the PMIC, and switch to a mode called "Push/Pull 2 Wire Interface". That bus is using SDA and SCL, with the start and stop conditions exactly like I2C does, but: - Once the start condition has been issued, the address isn't sent, only a direction bit. Hence, it does not support multiple devices anymore. - Once that direction bit has been sent, the master sends the register it wants to read from/write to, over 8 bits, followed by one parity bit. - Whenever you're writing, the master then sends the data over 8 bits, followed once again by a parity bit. Then, and only then, an ACK is issued by the slave. - Whenever you're reading, the master then clocks SDL and the slave drives SDA for 8 bits plus 1 parity bit. If there was some kind of error, the slave will pull SDA up for 9 cycles, resulting in a parity error. Like with I2C though, since it is the only and last byte it's receiving, the master won't issue an ACK. Obviously, to go ahead with the PMIC support, we need to support this controller and bus first. I can't really decide whether it's within the scope of the allowed protocol deviations of I2C or if we should create a whole new bus for it. Since it's quite similar to and starts as an I2C bus, I'd go for the former, but I'm really not definitive about it, and wanted to have your feedback. Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[PATCH v4 1/2] i2c: sunxi: Change i2c compatibles
The Allwinner A10 compatibles were following a slightly different compatible
patterns than the rest of the SoCs for historical reasons. Move to the other
pattern for consistency across all Allwinner Socs.
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 2 +-
drivers/i2c/busses/i2c-mv64xxx.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index befd4fb4764f..5c30026921ae 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -5,7 +5,7 @@ Required properties :
- reg : Offset and length of the register set for the device
- compatible : Should be either:
- - "allwinner,sun4i-i2c"
+ - "allwinner,sun4i-a10-i2c"
- "allwinner,sun6i-a31-i2c"
- "marvell,mv64xxx-i2c"
- "marvell,mv78230-i2c"
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 203a5482a866..2f817b0978a2 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -692,7 +692,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = {
*
*/
static const struct of_device_id mv64xxx_i2c_of_match_table[] = {
- { .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i},
+ { .compatible = "allwinner,sun4i-a10-i2c", .data =
&mv64xxx_i2c_regs_sun4i},
{ .compatible = "allwinner,sun6i-a31-i2c", .data =
&mv64xxx_i2c_regs_sun4i},
{ .compatible = "marvell,mv64xxx-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{ .compatible = "marvell,mv78230-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
--
1.9.0
--
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 v4 0/2] i2c: sunxi: Change compatibles pattern
Hi, This is the fourth version of the i2c compatible changes. Changes from v4: - Added extra per-SoC compatibles to the DT for all SoC but the A10 Maxime Ripard (2): i2c: sunxi: Change i2c compatibles ARM: sunxi: dt: Convert to the new i2c compatibles Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 2 +- arch/arm/boot/dts/sun4i-a10.dtsi | 6 +++--- arch/arm/boot/dts/sun5i-a10s.dtsi | 6 +++--- arch/arm/boot/dts/sun5i-a13.dtsi | 6 +++--- arch/arm/boot/dts/sun7i-a20.dtsi | 10 +- drivers/i2c/busses/i2c-mv64xxx.c | 2 +- 6 files changed, 16 insertions(+), 16 deletions(-) -- 1.9.0 -- 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 v4 2/2] ARM: sunxi: dt: Convert to the new i2c compatibles
Switch the device tree to the new compatibles introduced in the i2c drivers
to have a common pattern accross all Allwinner SoCs.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun4i-a10.dtsi | 6 +++---
arch/arm/boot/dts/sun5i-a10s.dtsi | 6 +++---
arch/arm/boot/dts/sun5i-a13.dtsi | 6 +++---
arch/arm/boot/dts/sun7i-a20.dtsi | 10 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 27dc6ee8d0d6..6d3e39b8bb29 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -641,7 +641,7 @@
};
i2c0: i2c@01c2ac00 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -650,7 +650,7 @@
};
i2c1: i2c@01c2b000 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -659,7 +659,7 @@
};
i2c2: i2c@01c2b400 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi
b/arch/arm/boot/dts/sun5i-a10s.dtsi
index f34e0d85c9be..acf1e06113c6 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -519,7 +519,7 @@
i2c0: i2c@01c2ac00 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun5i-a10s-i2c",
"allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -530,7 +530,7 @@
i2c1: i2c@01c2b000 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun5i-a10s-i2c",
"allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -541,7 +541,7 @@
i2c2: i2c@01c2b400 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun5i-a10s-i2c",
"allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 0e9c239af689..85b71a735b96 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -461,7 +461,7 @@
};
i2c0: i2c@01c2ac00 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun5i-a13-i2c",
"allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -470,7 +470,7 @@
};
i2c1: i2c@01c2b000 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun5i-a13-i2c",
"allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -479,7 +479,7 @@
};
i2c2: i2c@01c2b400 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun5i-a13-i2c",
"allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun7i-a2
Re: [PATCH] i2c: mv64xxx: Fix reset controller handling
On Fri, Mar 28, 2014 at 08:49:00AM +0100, Wolfram Sang wrote: > On Mon, Mar 10, 2014 at 12:12:10PM +0100, Maxime Ripard wrote: > > The reset framework recently gained optional stubs when > > CONFIG_RESET_CONTROLLER > > is not selected. It also introduced a function reset_get_optional, that is > > also > > dummy-defined whenever the framework isn't enabled, for drivers that needs > > an > > optional reset controller. > > > > Switch to this function, since the mv64xxx driver is in this case. This also > > fixes a compilation breakage whenever the reset framework wasn't selected: > > > > drivers/i2c/busses/i2c-mv64xxx.c:771:2: error: implicit declaration of > > function 'devm_reset_control_get' > > > > While we're at it, remove the redundant test on dev.of_node surrounding the > > calls to reset framework functions, since it will either be a valid > > pointer, an > > error pointer in the case where we called reset_get_optional without an > > of_node > > pointer or if it failed, or NULL if we're not loaded through DT. > > > > Signed-off-by: Maxime Ripard > > Applied to for-next, thanks! Thanks for your patience, and sorry for the mess. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 2/2] ARM: sunxi: dt: Convert to the new i2c compatibles
On Thu, Mar 13, 2014 at 10:27:25PM +0100, Wolfram Sang wrote:
> > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
> > b/arch/arm/boot/dts/sun7i-a20.dtsi
> > index 4cc2f5f2ecad..c1fd510fd2c4 100644
> > --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> > @@ -762,7 +762,7 @@
> > };
> >
> > i2c0: i2c@01c2ac00 {
> > - compatible = "allwinner,sun4i-i2c";
> > + compatible = "allwinner,sun4i-a10-i2c";
>
> I still wonder why there is no "allwinner,sun4i-a20-i2c" entry first
> (even if there is no similar entry in the driver *yet*).
Ahh, I didn't understand it like that in your previous mail. Yes, it
makes sense.
I'll update the patches and resend.
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Sat, Mar 22, 2014 at 12:11:24PM +0100, Arnd Bergmann wrote: > On Friday 21 March 2014 20:17:39 Maxime Ripard wrote: > > On Fri, Mar 21, 2014 at 11:49:59AM -0400, Paul Gortmaker wrote: > > > On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux > > > wrote: > > > > On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: > > > >> On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux > > > >> wrote: > > > >> > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > > > >> > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > > >> > > exit_free_irq: > > > >> > > free_irq(drv_data->irq, drv_data); > > > >> > > exit_reset: > > > >> > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > > > >> > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > > > >> > > + !IS_ERR(drv_data->rstc)) > > > >> > > reset_control_assert(drv_data->rstc); > > > >> > > > > >> > Another question is... why do we need to check pd->dev.of_node here? > > > >> > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > > > >> > controller node, so drv_data->rstc is either going to be a valid > > > >> > pointer, or it's going to be an error pointer - neither > > > >> > reset_control_get() nor devm_reset_control_get return NULL. > > > >> > > > >> Following back on this as I was doing the patch, actually, > > > >> drv_data->rstc will be NULL if we're not probed by DT, and hence never > > > >> call reset_control_get, that would set an error pointer. > > > >> > > > >> But then, we can use IS_ERR_OR_NULL on drv_data->rstc. > > > > > > > > I think you can also move the devm_reset_control_get() into the main > > > > probe function: you're only checking for -EPROBE_DEFER from it to fail, > > > > allowing other errors to continue with the driver init. This means > > > > that on non-OF, devm_reset_control_get() will fail with -ENOENT. > > > > > > Looping linux-next into the CC since this is the cause of the failure > > > in orion5x_defconfig there, and no point in anyone else re-doing the > > > same bisect. > > > > I sent a fix for this that hasn't been picked up yet: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html > > > > IIRC, Wolfram's away until Monday, so I guess it will be merged some > > time next week. > > I think there is something wrong with an interface that makes you use > IS_ERR_OR_NULL(). If you are calling reset_control_get_optional(), that' > should not return an error when there is no reset controller listed > in the device tree. We should still have a way to propagate -EPROBE_DEFER, > or bail out if there is a reset controller but there is something wrong > with it, but otherwise I'd suggest just leaving NULL as a valid pointer > in drv_data->rstc and making sure that the reset controller functions > can just deal with a NULL argument, so you never have to check it again. Actually, it's not the reset framework but the driver itself that needs this. The framework will always return an error pointer here, but we won't ever call reset_control_get_optional if we are not probed with DT, and in that case, we will have NULL is data->rstc, hence why we need to use IS_ERR_OR_NULL. We should probably fix the reset functions, but maybe that can come later so that we have marvell's defconfig fixed? -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 21, 2014 at 11:49:59AM -0400, Paul Gortmaker wrote: > On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux > wrote: > > On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: > >> On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: > >> > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > >> > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > >> > > exit_free_irq: > >> > > free_irq(drv_data->irq, drv_data); > >> > > exit_reset: > >> > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > >> > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > >> > > + !IS_ERR(drv_data->rstc)) > >> > > reset_control_assert(drv_data->rstc); > >> > > >> > Another question is... why do we need to check pd->dev.of_node here? > >> > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > >> > controller node, so drv_data->rstc is either going to be a valid > >> > pointer, or it's going to be an error pointer - neither > >> > reset_control_get() nor devm_reset_control_get return NULL. > >> > >> Following back on this as I was doing the patch, actually, > >> drv_data->rstc will be NULL if we're not probed by DT, and hence never > >> call reset_control_get, that would set an error pointer. > >> > >> But then, we can use IS_ERR_OR_NULL on drv_data->rstc. > > > > I think you can also move the devm_reset_control_get() into the main > > probe function: you're only checking for -EPROBE_DEFER from it to fail, > > allowing other errors to continue with the driver init. This means > > that on non-OF, devm_reset_control_get() will fail with -ENOENT. > > Looping linux-next into the CC since this is the cause of the failure > in orion5x_defconfig there, and no point in anyone else re-doing the > same bisect. I sent a fix for this that hasn't been picked up yet: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html IIRC, Wolfram's away until Monday, so I guess it will be merged some time next week. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 1/2] i2c: sunxi: Change i2c compatibles
On Thu, Mar 13, 2014 at 05:31:28PM +, Mark Rutland wrote: > On Thu, Mar 13, 2014 at 11:37:22AM +0000, Maxime Ripard wrote: > > The Allwinner A10 compatibles were following a slightly different compatible > > patterns than the rest of the SoCs for historical reasons. Move to the other > > pattern for consistency across all Allwinner Socs. > > Dropping support for the existing string breaks existing DTBs. Is > this really necessary? I added new ones at first, and Rob said it wasn't worth it since we were a "work-in-progress" platform. http://lists.infradead.org/pipermail/linux-arm-kernel/2014-February/229438.html -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[PATCH v3 0/2] i2c: sunxi: Change compatibles pattern
Hi, This is the third version of the i2c compatible changes. Changes from v2: - rebased on top of current i2c/for-next Maxime Ripard (2): i2c: sunxi: Change i2c compatibles ARM: sunxi: dt: Convert to the new i2c compatibles Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 2 +- arch/arm/boot/dts/sun4i-a10.dtsi | 6 +++--- arch/arm/boot/dts/sun5i-a10s.dtsi | 6 +++--- arch/arm/boot/dts/sun5i-a13.dtsi | 6 +++--- arch/arm/boot/dts/sun7i-a20.dtsi | 10 +- drivers/i2c/busses/i2c-mv64xxx.c | 2 +- 6 files changed, 16 insertions(+), 16 deletions(-) -- 1.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] ARM: sunxi: dt: Convert to the new i2c compatibles
Switch the device tree to the new compatibles introduced in the i2c drivers
to have a common pattern accross all Allwinner SoCs.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun4i-a10.dtsi | 6 +++---
arch/arm/boot/dts/sun5i-a10s.dtsi | 6 +++---
arch/arm/boot/dts/sun5i-a13.dtsi | 6 +++---
arch/arm/boot/dts/sun7i-a20.dtsi | 10 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 27dc6ee8d0d6..6d3e39b8bb29 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -641,7 +641,7 @@
};
i2c0: i2c@01c2ac00 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -650,7 +650,7 @@
};
i2c1: i2c@01c2b000 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -659,7 +659,7 @@
};
i2c2: i2c@01c2b400 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi
b/arch/arm/boot/dts/sun5i-a10s.dtsi
index f34e0d85c9be..4977f47aeab3 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -519,7 +519,7 @@
i2c0: i2c@01c2ac00 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -530,7 +530,7 @@
i2c1: i2c@01c2b000 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -541,7 +541,7 @@
i2c2: i2c@01c2b400 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 0e9c239af689..7e0ef8374bbf 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -461,7 +461,7 @@
};
i2c0: i2c@01c2ac00 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -470,7 +470,7 @@
};
i2c1: i2c@01c2b000 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -479,7 +479,7 @@
};
i2c2: i2c@01c2b400 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 4cc2f5f2ecad..c1fd510fd2c4 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -762,7 +762,7 @@
};
i2c0: i2c@01
[PATCH v3 1/2] i2c: sunxi: Change i2c compatibles
The Allwinner A10 compatibles were following a slightly different compatible
patterns than the rest of the SoCs for historical reasons. Move to the other
pattern for consistency across all Allwinner Socs.
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 2 +-
drivers/i2c/busses/i2c-mv64xxx.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index befd4fb4764f..5c30026921ae 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -5,7 +5,7 @@ Required properties :
- reg : Offset and length of the register set for the device
- compatible : Should be either:
- - "allwinner,sun4i-i2c"
+ - "allwinner,sun4i-a10-i2c"
- "allwinner,sun6i-a31-i2c"
- "marvell,mv64xxx-i2c"
- "marvell,mv78230-i2c"
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 203a5482a866..2f817b0978a2 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -692,7 +692,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = {
*
*/
static const struct of_device_id mv64xxx_i2c_of_match_table[] = {
- { .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i},
+ { .compatible = "allwinner,sun4i-a10-i2c", .data =
&mv64xxx_i2c_regs_sun4i},
{ .compatible = "allwinner,sun6i-a31-i2c", .data =
&mv64xxx_i2c_regs_sun4i},
{ .compatible = "marvell,mv64xxx-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{ .compatible = "marvell,mv78230-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
--
1.9.0
--
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: mv64xxx: Fix reset controller handling
The reset framework recently gained optional stubs when CONFIG_RESET_CONTROLLER
is not selected. It also introduced a function reset_get_optional, that is also
dummy-defined whenever the framework isn't enabled, for drivers that needs an
optional reset controller.
Switch to this function, since the mv64xxx driver is in this case. This also
fixes a compilation breakage whenever the reset framework wasn't selected:
drivers/i2c/busses/i2c-mv64xxx.c:771:2: error: implicit declaration of function
'devm_reset_control_get'
While we're at it, remove the redundant test on dev.of_node surrounding the
calls to reset framework functions, since it will either be a valid pointer, an
error pointer in the case where we called reset_get_optional without an of_node
pointer or if it failed, or NULL if we're not loaded through DT.
Signed-off-by: Maxime Ripard
---
Obviously, this patch depends on Philipp Zabel's "reset: Add optional resets
and stubs" patch that he merged and will send a pull request for.
drivers/i2c/busses/i2c-mv64xxx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 203a5482a866..a09af20342ed 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -768,7 +768,7 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
}
drv_data->irq = irq_of_parse_and_map(np, 0);
- drv_data->rstc = devm_reset_control_get(dev, NULL);
+ drv_data->rstc = devm_reset_control_get_optional(dev, NULL);
if (IS_ERR(drv_data->rstc)) {
if (PTR_ERR(drv_data->rstc) == -EPROBE_DEFER) {
rc = -EPROBE_DEFER;
@@ -900,7 +900,7 @@ mv64xxx_i2c_probe(struct platform_device *pd)
exit_free_irq:
free_irq(drv_data->irq, drv_data);
exit_reset:
- if (pd->dev.of_node && !IS_ERR(drv_data->rstc))
+ if (!IS_ERR_OR_NULL(drv_data->rstc))
reset_control_assert(drv_data->rstc);
exit_clk:
#if defined(CONFIG_HAVE_CLK)
@@ -920,7 +920,7 @@ mv64xxx_i2c_remove(struct platform_device *dev)
i2c_del_adapter(&drv_data->adapter);
free_irq(drv_data->irq, drv_data);
- if (dev->dev.of_node && !IS_ERR(drv_data->rstc))
+ if (!IS_ERR_OR_NULL(drv_data->rstc))
reset_control_assert(drv_data->rstc);
#if defined(CONFIG_HAVE_CLK)
/* Not all platforms have a clk */
--
1.9.0
--
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: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > exit_free_irq: > > free_irq(drv_data->irq, drv_data); > > exit_reset: > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > > + !IS_ERR(drv_data->rstc)) > > reset_control_assert(drv_data->rstc); > > Another question is... why do we need to check pd->dev.of_node here? > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > controller node, so drv_data->rstc is either going to be a valid > pointer, or it's going to be an error pointer - neither > reset_control_get() nor devm_reset_control_get return NULL. Following back on this as I was doing the patch, actually, drv_data->rstc will be NULL if we're not probed by DT, and hence never call reset_control_get, that would set an error pointer. But then, we can use IS_ERR_OR_NULL on drv_data->rstc. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 06:29:49PM +0100, Wolfram Sang wrote: > > > > Another question is... why do we need to check pd->dev.of_node here? > > > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > > > controller node, so drv_data->rstc is either going to be a valid > > > pointer, or it's going to be an error pointer - neither > > > reset_control_get() nor devm_reset_control_get return NULL. > > > > Hmmm, right. I'll fix this in a later version. > > > > Wolfram, do you want me to respin the patch making use of > > reset_get_optional introduced by Philip in its other mail? > > I think I'd prefer both issues fixed with one patch like in "fixing up > reset controller handling". You mean the of_node check and the use of reset_control_get_optional, right? > And you might want to give a Tested- or Reviewed-by tag to Philipp's > patch if you are going to use it. Yes, I will. I'll only have access to the hardware on monday though, so I won't be able to actually test it before then. Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > exit_free_irq: > > free_irq(drv_data->irq, drv_data); > > exit_reset: > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > > + !IS_ERR(drv_data->rstc)) > > reset_control_assert(drv_data->rstc); > > Another question is... why do we need to check pd->dev.of_node here? > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > controller node, so drv_data->rstc is either going to be a valid > pointer, or it's going to be an error pointer - neither > reset_control_get() nor devm_reset_control_get return NULL. Hmmm, right. I'll fix this in a later version. Wolfram, do you want me to respin the patch making use of reset_get_optional introduced by Philip in its other mail? -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[PATCH] i2c: mv64xxx: Fix compilation breakage
Commit 370136bc67c3 ("i2c: mv64xxx: Add reset deassert call"), introduced a
recursive dependency, which was fixed by commit 80c69915e5fb ("i2c: mv64xxx:
fix circular Kconfig dependency", that in turn, by dropping the dependency on
RESET_CONTROLLER, introduced a compilation breakage whenever this option wasn't
set.
drivers/i2c/busses/i2c-mv64xxx.c:924: undefined reference to
`reset_control_assert'
drivers/i2c/busses/i2c-mv64xxx.c:904: undefined reference to
`reset_control_assert'
drivers/i2c/busses/i2c-mv64xxx.c:771: undefined reference to
`devm_reset_control_get'
drivers/i2c/busses/i2c-mv64xxx.c:778: undefined reference to
`reset_control_deassert'
Since the reset framework doesn't define dummy stubs whenever
CONFIG_RESET_CONTROLLER is not defined, protect the reset framework calls by
IS_ENABLED tests to make sure it won't be compiled in.
Signed-off-by: Maxime Ripard
---
drivers/i2c/busses/i2c-mv64xxx.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 203a548..a1dc99b 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -768,14 +768,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
}
drv_data->irq = irq_of_parse_and_map(np, 0);
- drv_data->rstc = devm_reset_control_get(dev, NULL);
- if (IS_ERR(drv_data->rstc)) {
- if (PTR_ERR(drv_data->rstc) == -EPROBE_DEFER) {
- rc = -EPROBE_DEFER;
- goto out;
+ if (IS_ENABLED(CONFIG_RESET_CONTROLLER)) {
+ drv_data->rstc = devm_reset_control_get(dev, NULL);
+ if (IS_ERR(drv_data->rstc)) {
+ if (PTR_ERR(drv_data->rstc) == -EPROBE_DEFER) {
+ rc = -EPROBE_DEFER;
+ goto out;
+ }
+ } else {
+ reset_control_deassert(drv_data->rstc);
}
- } else {
- reset_control_deassert(drv_data->rstc);
}
/* Its not yet defined how timeouts will be specified in device tree.
@@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd)
exit_free_irq:
free_irq(drv_data->irq, drv_data);
exit_reset:
- if (pd->dev.of_node && !IS_ERR(drv_data->rstc))
+ if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) &&
+ !IS_ERR(drv_data->rstc))
reset_control_assert(drv_data->rstc);
exit_clk:
#if defined(CONFIG_HAVE_CLK)
@@ -920,7 +923,8 @@ mv64xxx_i2c_remove(struct platform_device *dev)
i2c_del_adapter(&drv_data->adapter);
free_irq(drv_data->irq, drv_data);
- if (dev->dev.of_node && !IS_ERR(drv_data->rstc))
+ if (dev->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) &&
+ !IS_ERR(drv_data->rstc))
reset_control_assert(drv_data->rstc);
#if defined(CONFIG_HAVE_CLK)
/* Not all platforms have a clk */
--
1.9.0
--
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: mv64xxx: Fix circular dependencies warning and compilation breakage
On Fri, Mar 07, 2014 at 03:24:05PM +0100, Wolfram Sang wrote:
>
> New patch -> new thread, please.
>
> > This patch fixes the circular dependency introduced by commit 370136bc67c3
> > ("i2c: mv64xxx: Add reset deassert call"):
> >
> > drivers/video/Kconfig:42:error: recursive dependency detected!
>
> Please base it on i2c-next. I already applied my patch.
> Your patch fixes the build error discovered by it.
I didn't realise that. I will.
> > Since the reset framework doesn't define dummy stubs whenever
> > CONFIG_RESET_CONTROLLER is not defined, it's the only sane way to have a
> > driver
> > that compiles in any cases.
>
> Paragraph needs reformat. And please drop "sane". #ifdefs are not sane.
> Fixing the reset framework would be sane.
Ok..
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [linux-sunxi] Re: [PATCH v3 1/5] i2c: mv64xxx: Add reset deassert call
Hi, On Fri, Mar 07, 2014 at 12:18:58PM +0100, Wolfram Sang wrote: > > > > Since RESET_CONTROLLER is not required for those platforms, it really > > > should be optional - and I think the real fix is for the reset controller > > > support to provide stub functions. > > > > Philipp Zabel suggested that adding a _optional variant that provides stubs > > and doesn't depend on RESET_CONTROLLER is probably better. This keeps the > > compile time checks for drivers requiring it. > > > > See: https://lkml.org/lkml/2014/1/10/220 > > > > I ended up dropping my patch though. > > Thanks for the pointer. Well, looks like I need to revert the offending > i2c patches then until this issue is fixed? We can't have > RESET_CONTROLLER (circular dependency) and we can't skip it (build > failures). > I just sent a fix in reply to your mail that should fix the issue without having to revert the patches. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[PATCH] i2c: mv64xxx: Fix circular dependencies warning and compilation breakage
This patch fixes the circular dependency introduced by commit 370136bc67c3
("i2c: mv64xxx: Add reset deassert call"):
drivers/video/Kconfig:42:error: recursive dependency detected!
Since the reset framework doesn't define dummy stubs whenever
CONFIG_RESET_CONTROLLER is not defined, it's the only sane way to have a driver
that compiles in any cases.
Signed-off-by: Maxime Ripard
---
drivers/i2c/busses/Kconfig | 1 -
drivers/i2c/busses/i2c-mv64xxx.c | 22 +-
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 70bcad9..f5ed031 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -528,7 +528,6 @@ config I2C_MPC
config I2C_MV64XXX
tristate "Marvell mv64xxx I2C Controller"
depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI)
- select RESET_CONTROLLER
help
If you say yes to this option, support will be included for the
built-in I2C interface on the Marvell 64xxx line of host bridges.
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 203a548..a1dc99b 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -768,14 +768,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
}
drv_data->irq = irq_of_parse_and_map(np, 0);
- drv_data->rstc = devm_reset_control_get(dev, NULL);
- if (IS_ERR(drv_data->rstc)) {
- if (PTR_ERR(drv_data->rstc) == -EPROBE_DEFER) {
- rc = -EPROBE_DEFER;
- goto out;
+ if (IS_ENABLED(CONFIG_RESET_CONTROLLER)) {
+ drv_data->rstc = devm_reset_control_get(dev, NULL);
+ if (IS_ERR(drv_data->rstc)) {
+ if (PTR_ERR(drv_data->rstc) == -EPROBE_DEFER) {
+ rc = -EPROBE_DEFER;
+ goto out;
+ }
+ } else {
+ reset_control_deassert(drv_data->rstc);
}
- } else {
- reset_control_deassert(drv_data->rstc);
}
/* Its not yet defined how timeouts will be specified in device tree.
@@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd)
exit_free_irq:
free_irq(drv_data->irq, drv_data);
exit_reset:
- if (pd->dev.of_node && !IS_ERR(drv_data->rstc))
+ if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) &&
+ !IS_ERR(drv_data->rstc))
reset_control_assert(drv_data->rstc);
exit_clk:
#if defined(CONFIG_HAVE_CLK)
@@ -920,7 +923,8 @@ mv64xxx_i2c_remove(struct platform_device *dev)
i2c_del_adapter(&drv_data->adapter);
free_irq(drv_data->irq, drv_data);
- if (dev->dev.of_node && !IS_ERR(drv_data->rstc))
+ if (dev->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) &&
+ !IS_ERR(drv_data->rstc))
reset_control_assert(drv_data->rstc);
#if defined(CONFIG_HAVE_CLK)
/* Not all platforms have a clk */
--
1.9.0
--
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 2/5] i2c: mv64xxx: Add support for the Allwinner A31 I2C driver
Hi Wolfram, On Wed, Mar 05, 2014 at 05:34:30PM +0100, Wolfram Sang wrote: > On Tue, Mar 04, 2014 at 05:28:38PM +0100, Maxime Ripard wrote: > > The Allwinner A31 I2C controller is almost identical to the one used in the > > other Allwinner SoCs, except for the fact that it needs to clear the > > interrupt > > by setting the INT_FLAGS bit in the control register, instead of clearing > > it. > > > > Signed-off-by: Maxime Ripard > > Reviewed-by: Gregory CLEMENT > > Tested-by: Gregory CLEMENT > > Applied to for-next, thanks! > > Still... > > > + if (of_device_is_compatible(np, "allwinner,sun6i-a31-i2c")) > > + drv_data->irq_clear_inverted = true; > > ... next time an errata is needed, I think it makes sense to refactor all > these checks into one struct which can be used as match->data directly. Yep, it makes sense. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 1/5] i2c: mv64xxx: Add reset deassert call
Hi Russell, On Fri, Mar 07, 2014 at 09:52:23AM +, Russell King - ARM Linux wrote: > On Tue, Mar 04, 2014 at 05:28:37PM +0100, Maxime Ripard wrote: > > The Allwinner A31 SoC using that IP has a reset controller maintaining > > it reset unless told otherwise. > > > > Add some optional reset support to the driver. > > > > Signed-off-by: Maxime Ripard > > Reviewed-by: Gregory CLEMENT > > Tested-by: Gregory CLEMENT > > This appears to be causing some build errors in Olof's next builder > for many of the ARM platforms which make use of this: > > drivers/i2c/busses/i2c-mv64xxx.c:924: undefined reference to > `reset_control_assert' > drivers/i2c/busses/i2c-mv64xxx.c:904: undefined reference to > `reset_control_assert' > drivers/i2c/busses/i2c-mv64xxx.c:771: undefined reference to > `devm_reset_control_get' > drivers/i2c/busses/i2c-mv64xxx.c:778: undefined reference to > `reset_control_deassert' The reset framework doesn't define its functions when its not selected, and somehow I think it was not here. What's odd is that there is an explicit select on RESET_CONTROLLER in the Kconfig. Maybe it's the circular dependency issue that has been reported that cause this and Wolfram sent a patch for: http://patchwork.ozlabs.org/patch/327573/ Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH V2] i2c: mv64xxx: fix circular Kconfig dependency
Hi Wolfram,
On Thu, Mar 06, 2014 at 09:32:55PM +0100, Wolfram Sang wrote:
> Commit 370136bc67c3 ("i2c: mv64xxx: Add reset deassert call")
> introduced:
>
> drivers/video/Kconfig:42:error: recursive dependency detected!
>
> ARCH_SUNXI selects RESET_CONTROLLER anyhow.
>
> Signed-off-by: Wolfram Sang
> ---
> drivers/i2c/busses/Kconfig | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 70bcad941657..f8162441576f 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -527,8 +527,7 @@ config I2C_MPC
>
> config I2C_MV64XXX
> tristate "Marvell mv64xxx I2C Controller"
> - depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI)
> - select RESET_CONTROLLER
> + depends on MV64X60 || PLAT_ORION || ARCH_SUNXI
I'm not sure it actually works.
MV64X60 and PLAT_ORION don't select RESET_CONTROLLER, and the
functions won't be declared, resulting in a compilation breakage too.
Maybe we can just depend on RESET_CONTROLLER too here, to fix the
circular dependency.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
[PATCH v3 5/5] ARM: sun6i: colombus: Enable the I2C controllers
The A31 Colombus board has 3 I2C controllers that should be usable. However,
the first one is not working for some reason on the hardware I have been able
to test it on, while it should really be the same controller. Enable the i2c1
and i2c2 busses, and mark i2c0 as in failure in the DT.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun6i-a31-colombus.dts | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-colombus.dts
b/arch/arm/boot/dts/sun6i-a31-colombus.dts
index e5adae3..3898a7b 100644
--- a/arch/arm/boot/dts/sun6i-a31-colombus.dts
+++ b/arch/arm/boot/dts/sun6i-a31-colombus.dts
@@ -28,5 +28,23 @@
pinctrl-0 = <&uart0_pins_a>;
status = "okay";
};
+
+ i2c0: i2c@01c2ac00 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins_a>;
+ status = "fail";
+ };
+
+ i2c1: i2c@01c2b000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c1_pins_a>;
+ status = "okay";
+ };
+
+ i2c2: i2c@01c2b400 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_pins_a>;
+ status = "okay";
+ };
};
};
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/5] i2c: mv64xxx: Add support for the Allwinner A31 I2C driver
The Allwinner A31 I2C controller is almost identical to the one used in the
other Allwinner SoCs, except for the fact that it needs to clear the interrupt
by setting the INT_FLAGS bit in the control register, instead of clearing it.
Signed-off-by: Maxime Ripard
Reviewed-by: Gregory CLEMENT
Tested-by: Gregory CLEMENT
---
.../devicetree/bindings/i2c/i2c-mv64xxx.txt | 21 ++---
drivers/i2c/busses/i2c-mv64xxx.c| 11 +++
2 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index 21062bc..befd4fb 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -4,19 +4,26 @@
Required properties :
- reg : Offset and length of the register set for the device
- - compatible : Should be "marvell,mv64xxx-i2c" or "allwinner,sun4i-i2c"
- or "marvell,mv78230-i2c" or "marvell,mv78230-a0-i2c"
- Note: Only use "marvell,mv78230-a0-i2c" for a very rare,
- initial version of the SoC which had broken offload
- support. Linux auto-detects this and sets it
- appropriately.
+ - compatible : Should be either:
+ - "allwinner,sun4i-i2c"
+ - "allwinner,sun6i-a31-i2c"
+ - "marvell,mv64xxx-i2c"
+ - "marvell,mv78230-i2c"
+ - "marvell,mv78230-a0-i2c"
+ * Note: Only use "marvell,mv78230-a0-i2c" for a
+ very rare, initial version of the SoC which
+ had broken offload support. Linux
+ auto-detects this and sets it appropriately.
- interrupts : The interrupt number
Optional properties :
- clock-frequency : Desired I2C bus clock frequency in Hz. If not set the
default frequency is 100kHz
- - resets : phandle to the parent reset controller
+
+ - resets : phandle to the parent reset controller. Mandatory
+ whenever you're using the "allwinner,sun6i-a31-i2c"
+ compatible.
Examples:
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 1bb69b6..203a548 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -150,6 +150,7 @@ struct mv64xxx_i2c_data {
/* 5us delay in order to avoid repeated start timing violation */
boolerrata_delay;
struct reset_control*rstc;
+ boolirq_clear_inverted;
};
static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
@@ -568,6 +569,11 @@ mv64xxx_i2c_intr(int irq, void *dev_id)
status = readl(drv_data->reg_base +
drv_data->reg_offsets.status);
mv64xxx_i2c_fsm(drv_data, status);
mv64xxx_i2c_do_action(drv_data);
+
+ if (drv_data->irq_clear_inverted)
+ writel(drv_data->cntl_bits |
MV64XXX_I2C_REG_CONTROL_IFLG,
+ drv_data->reg_base +
drv_data->reg_offsets.control);
+
rc = IRQ_HANDLED;
}
spin_unlock_irqrestore(&drv_data->lock, flags);
@@ -687,6 +693,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = {
*/
static const struct of_device_id mv64xxx_i2c_of_match_table[] = {
{ .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i},
+ { .compatible = "allwinner,sun6i-a31-i2c", .data =
&mv64xxx_i2c_regs_sun4i},
{ .compatible = "marvell,mv64xxx-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{ .compatible = "marvell,mv78230-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{ .compatible = "marvell,mv78230-a0-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
@@ -795,6 +802,10 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
drv_data->offload_enabled = false;
drv_data->errata_delay = true;
}
+
+ if (of_device_is_compatible(np, "allwinner,sun6i-a31-i2c"))
+ drv_data->irq_clear_inverted = true;
+
out:
return rc;
#endif
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/5] ARM: sun6i: Add support for the A31 I2C controller
Hi everyone, This patchset adds support the A31 i2c controller. This is mostly the same controller as the one found in the other Allwinner SoCs, except for the interrupts acking. On the other SoCs using this driver, the interrupts are acked by clearing the INT_FLAG bit in the control register, while on the A31, the interrupt is acked by writing that bit into the control register. The other difference is that the I2C IP is maintained in reset by a reset controller, so we're adding optionnal support for the reset framework in the driver to deassert the device from reset. Thanks! Maxime Changes from v2: - Rebased on top of v3.14-rc5 - Added Gregory Clement's Reviewed-by and Tested-by tags Changes from v1: - Handle EPROBE_DEFER from the reset framework - Put the device back in reset at remove/failed probe - Document the newly introduced compatible string Maxime Ripard (5): i2c: mv64xxx: Add reset deassert call i2c: mv64xxx: Add support for the Allwinner A31 I2C driver ARM: sun6i: Enable the I2C controllers ARM: sun6i: Enable the I2C muxing options ARM: sun6i: colombus: Enable the I2C controllers .../devicetree/bindings/i2c/i2c-mv64xxx.txt| 20 --- arch/arm/boot/dts/sun6i-a31-colombus.dts | 18 +++ arch/arm/boot/dts/sun6i-a31.dtsi | 61 ++ drivers/i2c/busses/Kconfig | 1 + drivers/i2c/busses/i2c-mv64xxx.c | 32 +++- 5 files changed, 124 insertions(+), 8 deletions(-) -- 1.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/5] ARM: sun6i: Enable the I2C controllers
The A31 has 4 I2C controllers that are the same than the one in the
other Allwinner SoCs, except for the fact that they are asserted in
reset by the reset unit.
Add these i2c controllers to the DTSI.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun6i-a31.dtsi | 40
1 file changed, 40 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index af6f87c..22d3eae 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -359,6 +359,46 @@
status = "disabled";
};
+ i2c0: i2c@01c2ac00 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2ac00 0x400>;
+ interrupts = <0 6 4>;
+ clocks = <&apb2_gates 0>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 0>;
+ status = "disabled";
+ };
+
+ i2c1: i2c@01c2b000 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2b000 0x400>;
+ interrupts = <0 7 4>;
+ clocks = <&apb2_gates 1>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 1>;
+ status = "disabled";
+ };
+
+ i2c2: i2c@01c2b400 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2b400 0x400>;
+ interrupts = <0 8 4>;
+ clocks = <&apb2_gates 2>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 2>;
+ status = "disabled";
+ };
+
+ i2c3: i2c@01c2b800 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2b800 0x400>;
+ interrupts = <0 9 4>;
+ clocks = <&apb2_gates 3>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 3>;
+ status = "disabled";
+ };
+
spi0: spi@01c68000 {
compatible = "allwinner,sun6i-a31-spi";
reg = <0x01c68000 0x1000>;
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/5] ARM: sun6i: Enable the I2C muxing options
The i2c controllers have a few muxing options on the A31. Enable the
ones found in the A31 Colombus board.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun6i-a31.dtsi | 21 +
1 file changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 22d3eae..8d3ea29 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -257,6 +257,27 @@
allwinner,drive = <0>;
allwinner,pull = <0>;
};
+
+ i2c0_pins_a: i2c0@0 {
+ allwinner,pins = "PH14", "PH15";
+ allwinner,function = "i2c0";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c1_pins_a: i2c1@0 {
+ allwinner,pins = "PH16", "PH17";
+ allwinner,function = "i2c1";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c2_pins_a: i2c2@0 {
+ allwinner,pins = "PH18", "PH19";
+ allwinner,function = "i2c2";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
};
ahb1_rst: reset@01c202c0 {
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/5] i2c: mv64xxx: Add reset deassert call
The Allwinner A31 SoC using that IP has a reset controller maintaining
it reset unless told otherwise.
Add some optional reset support to the driver.
Signed-off-by: Maxime Ripard
Reviewed-by: Gregory CLEMENT
Tested-by: Gregory CLEMENT
---
.../devicetree/bindings/i2c/i2c-mv64xxx.txt | 1 +
drivers/i2c/busses/Kconfig | 1 +
drivers/i2c/busses/i2c-mv64xxx.c| 21 +++--
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index 582b465..21062bc 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -16,6 +16,7 @@ Optional properties :
- clock-frequency : Desired I2C bus clock frequency in Hz. If not set the
default frequency is 100kHz
+ - resets : phandle to the parent reset controller
Examples:
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index f5ed031..70bcad9 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -528,6 +528,7 @@ config I2C_MPC
config I2C_MV64XXX
tristate "Marvell mv64xxx I2C Controller"
depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI)
+ select RESET_CONTROLLER
help
If you say yes to this option, support will be included for the
built-in I2C interface on the Marvell 64xxx line of host bridges.
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index d52d849..1bb69b6 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -148,6 +149,7 @@ struct mv64xxx_i2c_data {
booloffload_enabled;
/* 5us delay in order to avoid repeated start timing violation */
boolerrata_delay;
+ struct reset_control*rstc;
};
static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
@@ -759,6 +761,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
}
drv_data->irq = irq_of_parse_and_map(np, 0);
+ drv_data->rstc = devm_reset_control_get(dev, NULL);
+ if (IS_ERR(drv_data->rstc)) {
+ if (PTR_ERR(drv_data->rstc) == -EPROBE_DEFER) {
+ rc = -EPROBE_DEFER;
+ goto out;
+ }
+ } else {
+ reset_control_deassert(drv_data->rstc);
+ }
+
/* Its not yet defined how timeouts will be specified in device tree.
* So hard code the value to 1 second.
*/
@@ -845,7 +857,7 @@ mv64xxx_i2c_probe(struct platform_device *pd)
}
if (drv_data->irq < 0) {
rc = -ENXIO;
- goto exit_clk;
+ goto exit_reset;
}
drv_data->adapter.dev.parent = &pd->dev;
@@ -865,7 +877,7 @@ mv64xxx_i2c_probe(struct platform_device *pd)
dev_err(&drv_data->adapter.dev,
"mv64xxx: Can't register intr handler irq%d: %d\n",
drv_data->irq, rc);
- goto exit_clk;
+ goto exit_reset;
} else if ((rc = i2c_add_numbered_adapter(&drv_data->adapter)) != 0) {
dev_err(&drv_data->adapter.dev,
"mv64xxx: Can't add i2c adapter, rc: %d\n", -rc);
@@ -876,6 +888,9 @@ mv64xxx_i2c_probe(struct platform_device *pd)
exit_free_irq:
free_irq(drv_data->irq, drv_data);
+exit_reset:
+ if (pd->dev.of_node && !IS_ERR(drv_data->rstc))
+ reset_control_assert(drv_data->rstc);
exit_clk:
#if defined(CONFIG_HAVE_CLK)
/* Not all platforms have a clk */
@@ -894,6 +909,8 @@ mv64xxx_i2c_remove(struct platform_device *dev)
i2c_del_adapter(&drv_data->adapter);
free_irq(drv_data->irq, drv_data);
+ if (dev->dev.of_node && !IS_ERR(drv_data->rstc))
+ reset_control_assert(drv_data->rstc);
#if defined(CONFIG_HAVE_CLK)
/* Not all platforms have a clk */
if (!IS_ERR(drv_data->clk)) {
--
1.9.0
--
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 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller
Hi Bjorn,
On Thu, Feb 20, 2014 at 04:38:10PM -0800, Bjorn Andersson wrote:
> +static int qup_i2c_probe(struct platform_device *pdev)
> +{
[ snip ]
> +
> + qup_i2c_enable_clocks(qup);
> +
[ snip ]
> +
> + pm_runtime_set_autosuspend_delay(qup->dev, MSEC_PER_SEC);
> + pm_runtime_use_autosuspend(qup->dev);
> + pm_runtime_enable(qup->dev);
Since the device is already woken up, you probably need to call
pm_runtime_set_active here.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 0/5] mv64xxx updates
Hi, On Thu, Feb 13, 2014 at 09:51:00PM +0100, Thomas Petazzoni wrote: > Dear Wolfram Sang, > > On Thu, 13 Feb 2014 21:36:28 +0100, Wolfram Sang wrote: > > So, this is a series I came up with trying to fix the issue found by Kevin. > > Patches 1+2 are hopefully fixing the bug (in theory, I don't have the HW). > > Patches 3-5 are RFC, and if patch 3 actually works (see the CHECKME), then > > 4+5 > > are further cleanup possibilities. And there is still more potential, I > > mainly > > wanted to give some inspiration and awareness that the driver could need > > some > > more love. Please test at least 1+2, comments to 3-5 very welcome. > > > > Sorry for the delay, I got distracted by an NMI. > > I'm adding Maxime Ripard in Cc, since he is the maintainer of the > Allwinner platform, which also uses this I2C driver. So I guess he > would like to be notified of such changes in order to test that the > driver still works for him. I just gave a try to these patches, on my A31 board with the extra patches sent previously. Tested-by: Maxime Ripard Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 0/5] ARM: sun6i: Add support for the A31 I2C controller
On Mon, Feb 17, 2014 at 03:26:12PM +0100, Wolfram Sang wrote: > > > It's been over a month now, and no sign of life from you on this patch > > so far... Should I just take this patchset through my tree? :) > > Nope. Then please review those 40 lines of code. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 0/5] ARM: sun6i: Add support for the A31 I2C controller
Hi Wolfram, On Mon, Jan 13, 2014 at 11:34:48AM +0100, Maxime Ripard wrote: > Hi everyone, > > This patchset adds support the A31 i2c controller. This is mostly the > same controller as the one found in the other Allwinner SoCs, except > for the interrupts acking. > > On the other SoCs using this driver, the interrupts are acked by > clearing the INT_FLAG bit in the control register, while on the A31, > the interrupt is acked by writing that bit into the control register. > > The other difference is that the I2C IP is maintained in reset by a > reset controller, so we're adding optionnal support for the reset > framework in the driver to deassert the device from reset. It's been over a month now, and no sign of life from you on this patch so far... Should I just take this patchset through my tree? :) Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 2/2] ARM: sunxi: dt: Convert to the new i2c compatibles
On Fri, Feb 14, 2014 at 08:44:07AM +0100, Wolfram Sang wrote: > > > > For non-a10, That should be at least > > > > > > compatible = "allwinner,sun4i-a13-i2c", "allwinner,sun4i-a10-i2c"; > > > > > > or > > > > > > compatible = "allwinner,sun4i-a13-i2c", "allwinner,sun4i-i2c"; > > > > > > depending on the outcome above. > > > > > > Or is my knowledge outdated already? > > > > > > > Since they are strictly compatible, we don't need to introduce any > > different compatible string here. > > You never know all errata in advance. From what I know, one should > always use the specfic naming first, and then the generic fallback. So, > in case a distinction is needed later (think errata), then one doesn't > need to change the devicetrees. > And adding a A13-specific compatible wouldn't change anything, because it does work on at least one revision of them, so if you'd have to deal with an errata, you'd have to introduce a new compatible for this revision only anyway. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 2/2] ARM: sunxi: dt: Convert to the new i2c compatibles
On Thu, Feb 13, 2014 at 09:26:30AM +0100, Wolfram Sang wrote:
>
> Why is the devicetree list not on CC? (Added now)
>
> On Thu, Feb 06, 2014 at 10:51:25AM +0100, Maxime Ripard wrote:
> > Switch the device tree to the new compatibles introduced in the i2c drivers
> > to have a common pattern accross all Allwinner SoCs.
> >
> > Signed-off-by: Maxime Ripard
> > ---
> > arch/arm/boot/dts/sun4i-a10.dtsi | 6 +++---
> > arch/arm/boot/dts/sun5i-a10s.dtsi | 6 +++---
> > arch/arm/boot/dts/sun5i-a13.dtsi | 6 +++---
> > arch/arm/boot/dts/sun7i-a20.dtsi | 10 +-
> > 4 files changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi
> > b/arch/arm/boot/dts/sun4i-a10.dtsi
> > index 28273f9..ac65c8a 100644
> > --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> > +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> > @@ -518,7 +518,7 @@
> > };
> >
> > i2c0: i2c@01c2ac00 {
> > - compatible = "allwinner,sun4i-i2c";
> > + compatible = "allwinner,sun4i-a10-i2c";
>
> Can't we have:
>
> compatible = "allwinner,sun4i-a10-i2c", "allwinner,sun4i-i2c";
>
> ? And keep the old "allwinner,sun4i-i2c" and extend it with a SoC
> specific a10 compatible entry when a distinction is needed?
Actually, the two are exactly equivalent. The point is that the
compatible naming scheme doesn't follow what we are using (which is
--i2c), so we wan't to get rid of the old naming scheme
all together.
> > diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi
> > b/arch/arm/boot/dts/sun5i-a13.dtsi
> > index 6de40b6..537072c 100644
> > --- a/arch/arm/boot/dts/sun5i-a13.dtsi
> > +++ b/arch/arm/boot/dts/sun5i-a13.dtsi
> > @@ -377,7 +377,7 @@
> > };
> >
> > i2c0: i2c@01c2ac00 {
> > - compatible = "allwinner,sun4i-i2c";
> > + compatible = "allwinner,sun4i-a10-i2c";
>
> For non-a10, That should be at least
>
> compatible = "allwinner,sun4i-a13-i2c", "allwinner,sun4i-a10-i2c";
>
> or
>
> compatible = "allwinner,sun4i-a13-i2c", "allwinner,sun4i-i2c";
>
> depending on the outcome above.
>
> Or is my knowledge outdated already?
>
Since they are strictly compatible, we don't need to introduce any
different compatible string here.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH v2 0/2] i2c: sunxi: Change compatibles pattern
On Thu, Feb 13, 2014 at 09:19:36AM +0100, Wolfram Sang wrote: > > > This is the second version of the i2c compatible changes. > > The only difference with the v1 being that we're now droping the old > > compatibles, instead of keeping them, since the DT maintainers said it was > > fine. > > Please provide a pointer to that discussion. > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-February/229438.html -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v2 0/5] ARM: sun6i: Add support for the A31 I2C controller
On Mon, Jan 27, 2014 at 04:03:12PM +0100, Maxime Ripard wrote: > Hi Wolfram, > > On Mon, Jan 13, 2014 at 11:34:48AM +0100, Maxime Ripard wrote: > > Hi everyone, > > > > This patchset adds support the A31 i2c controller. This is mostly the > > same controller as the one found in the other Allwinner SoCs, except > > for the interrupts acking. > > > > On the other SoCs using this driver, the interrupts are acked by > > clearing the INT_FLAG bit in the control register, while on the A31, > > the interrupt is acked by writing that bit into the control register. > > > > The other difference is that the I2C IP is maintained in reset by a > > reset controller, so we're adding optionnal support for the reset > > framework in the driver to deassert the device from reset. > > Do you have any comments on this? Ping? -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[PATCH v2 1/2] i2c: sunxi: Change i2c compatibles
The Allwinner A10 compatibles were following a slightly different compatible
patterns than the rest of the SoCs for historical reasons. Move to the other
pattern for consistency across all Allwinner Socs.
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 2 +-
drivers/i2c/busses/i2c-mv64xxx.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index 582b465..5f4c0c8 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -4,7 +4,7 @@
Required properties :
- reg : Offset and length of the register set for the device
- - compatible : Should be "marvell,mv64xxx-i2c" or "allwinner,sun4i-i2c"
+ - compatible : Should be "marvell,mv64xxx-i2c" or
"allwinner,sun4i-a10-i2c"
or "marvell,mv78230-i2c" or "marvell,mv78230-a0-i2c"
Note: Only use "marvell,mv78230-a0-i2c" for a very rare,
initial version of the SoC which had broken offload
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index b8c5187..eb72301 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -689,7 +689,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = {
*
*/
static const struct of_device_id mv64xxx_i2c_of_match_table[] = {
- { .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i},
+ { .compatible = "allwinner,sun4i-a10-i2c", .data =
&mv64xxx_i2c_regs_sun4i},
{ .compatible = "marvell,mv64xxx-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{ .compatible = "marvell,mv78230-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{ .compatible = "marvell,mv78230-a0-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] ARM: sunxi: dt: Convert to the new i2c compatibles
Switch the device tree to the new compatibles introduced in the i2c drivers
to have a common pattern accross all Allwinner SoCs.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun4i-a10.dtsi | 6 +++---
arch/arm/boot/dts/sun5i-a10s.dtsi | 6 +++---
arch/arm/boot/dts/sun5i-a13.dtsi | 6 +++---
arch/arm/boot/dts/sun7i-a20.dtsi | 10 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 28273f9..ac65c8a 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -518,7 +518,7 @@
};
i2c0: i2c@01c2ac00 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -527,7 +527,7 @@
};
i2c1: i2c@01c2b000 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -536,7 +536,7 @@
};
i2c2: i2c@01c2b400 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi
b/arch/arm/boot/dts/sun5i-a10s.dtsi
index 2318082..a2005c7 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -435,7 +435,7 @@
i2c0: i2c@01c2ac00 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -446,7 +446,7 @@
i2c1: i2c@01c2b000 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -457,7 +457,7 @@
i2c2: i2c@01c2b400 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 6de40b6..537072c 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -377,7 +377,7 @@
};
i2c0: i2c@01c2ac00 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -386,7 +386,7 @@
};
i2c1: i2c@01c2b000 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -395,7 +395,7 @@
};
i2c2: i2c@01c2b400 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index bfb2cf2..ec4463f 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -564,7 +564,7 @@
};
i2c0: i2c@01c2ac00 {
- comp
[PATCH v2 0/2] i2c: sunxi: Change compatibles pattern
Hi, This is the second version of the i2c compatible changes. The only difference with the v1 being that we're now droping the old compatibles, instead of keeping them, since the DT maintainers said it was fine. Thanks, Maxime Maxime Ripard (2): i2c: sunxi: Change i2c compatibles ARM: sunxi: dt: Convert to the new i2c compatibles Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 2 +- arch/arm/boot/dts/sun4i-a10.dtsi | 6 +++--- arch/arm/boot/dts/sun5i-a10s.dtsi | 6 +++--- arch/arm/boot/dts/sun5i-a13.dtsi | 6 +++--- arch/arm/boot/dts/sun7i-a20.dtsi | 10 +- drivers/i2c/busses/i2c-mv64xxx.c | 2 +- 6 files changed, 16 insertions(+), 16 deletions(-) -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] i2c: sunxi: Add new i2c compatibles
The Allwinner A10 compatibles were following a slightly different compatible
patterns than the rest of the SoCs for historical reasons. Add compatibles
matching the other pattern to the i2c driver for consistency, and keep the
older one for backward compatibility.
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 4 ++--
drivers/i2c/busses/i2c-mv64xxx.c | 5 -
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index 82e8f6f..bd08d67 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -4,8 +4,8 @@
Required properties :
- reg : Offset and length of the register set for the device
- - compatible : Should be "marvell,mv64xxx-i2c" or "allwinner,sun4i-i2c"
- or "marvell,mv78230-i2c"
+ - compatible : Should be "marvell,mv64xxx-i2c" or
"allwinner,sun4i-a10-i2c"
+ or "marvell,mv78230-i2c" (Deprecated
"allwinner,sun4i-i2c")
- interrupts : The interrupt number
Optional properties :
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 8be7e42..e37a042 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -689,9 +689,12 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = {
*
*/
static const struct of_device_id mv64xxx_i2c_of_match_table[] = {
- { .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i},
+ { .compatible = "allwinner,sun4i-a10-i2c", .data =
&mv64xxx_i2c_regs_sun4i},
{ .compatible = "marvell,mv64xxx-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{ .compatible = "marvell,mv78230-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
+
+ /* Deprecated */
+ { .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i},
{}
};
MODULE_DEVICE_TABLE(of, mv64xxx_i2c_of_match_table);
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] ARM: sunxi: dt: Convert to the new i2c compatibles
Switch the device tree to the new compatibles introduced in the i2c drivers
to have a common pattern accross all Allwinner SoCs.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun4i-a10.dtsi | 6 +++---
arch/arm/boot/dts/sun5i-a10s.dtsi | 6 +++---
arch/arm/boot/dts/sun5i-a13.dtsi | 6 +++---
arch/arm/boot/dts/sun7i-a20.dtsi | 10 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 040bb0e..9508fb7 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -512,7 +512,7 @@
};
i2c0: i2c@01c2ac00 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -521,7 +521,7 @@
};
i2c1: i2c@01c2b000 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -530,7 +530,7 @@
};
i2c2: i2c@01c2b400 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi
b/arch/arm/boot/dts/sun5i-a10s.dtsi
index ea16054..0d1a3bf 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -431,7 +431,7 @@
i2c0: i2c@01c2ac00 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -442,7 +442,7 @@
i2c1: i2c@01c2b000 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -453,7 +453,7 @@
i2c2: i2c@01c2b400 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 320335a..9e6f031 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -372,7 +372,7 @@
};
i2c0: i2c@01c2ac00 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2ac00 0x400>;
interrupts = <7>;
clocks = <&apb1_gates 0>;
@@ -381,7 +381,7 @@
};
i2c1: i2c@01c2b000 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b000 0x400>;
interrupts = <8>;
clocks = <&apb1_gates 1>;
@@ -390,7 +390,7 @@
};
i2c2: i2c@01c2b400 {
- compatible = "allwinner,sun4i-i2c";
+ compatible = "allwinner,sun4i-a10-i2c";
reg = <0x01c2b400 0x400>;
interrupts = <9>;
clocks = <&apb1_gates 2>;
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 119f066..0a368d2 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -549,7 +549,7 @@
};
i2c0: i2c@01c2ac00 {
- comp
Re: [PATCH v2 0/5] ARM: sun6i: Add support for the A31 I2C controller
Hi Wolfram, On Mon, Jan 13, 2014 at 11:34:48AM +0100, Maxime Ripard wrote: > Hi everyone, > > This patchset adds support the A31 i2c controller. This is mostly the > same controller as the one found in the other Allwinner SoCs, except > for the interrupts acking. > > On the other SoCs using this driver, the interrupts are acked by > clearing the INT_FLAG bit in the control register, while on the A31, > the interrupt is acked by writing that bit into the control register. > > The other difference is that the I2C IP is maintained in reset by a > reset controller, so we're adding optionnal support for the reset > framework in the driver to deassert the device from reset. Do you have any comments on this? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[PATCH v2 4/5] ARM: sun6i: Enable the I2C muxing options
The i2c controllers have a few muxing options on the A31. Enable the
ones found in the A31 Colombus board.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun6i-a31.dtsi | 21 +
1 file changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 7dac496..668de00 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -210,6 +210,27 @@
allwinner,drive = <0>;
allwinner,pull = <0>;
};
+
+ i2c0_pins_a: i2c0@0 {
+ allwinner,pins = "PH14", "PH15";
+ allwinner,function = "i2c0";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c1_pins_a: i2c1@0 {
+ allwinner,pins = "PH16", "PH17";
+ allwinner,function = "i2c1";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c2_pins_a: i2c2@0 {
+ allwinner,pins = "PH18", "PH19";
+ allwinner,function = "i2c2";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
};
ahb1_rst: reset@01c202c0 {
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/5] ARM: sun6i: Add support for the A31 I2C controller
Hi everyone, This patchset adds support the A31 i2c controller. This is mostly the same controller as the one found in the other Allwinner SoCs, except for the interrupts acking. On the other SoCs using this driver, the interrupts are acked by clearing the INT_FLAG bit in the control register, while on the A31, the interrupt is acked by writing that bit into the control register. The other difference is that the I2C IP is maintained in reset by a reset controller, so we're adding optionnal support for the reset framework in the driver to deassert the device from reset. Thanks! Maxime Changes from v1: - Handle EPROBE_DEFER from the reset framework - Put the device back in reset at remove/failed probe - Document the newly introduced compatible string Maxime Ripard (5): i2c: mv64xxx: Add reset deassert call i2c: mv64xxx: Add support for the Allwinner A31 I2C driver ARM: sun6i: Enable the I2C controllers ARM: sun6i: Enable the I2C muxing options ARM: sun6i: colombus: Enable the I2C controllers .../devicetree/bindings/i2c/i2c-mv64xxx.txt| 8 ++- arch/arm/boot/dts/sun6i-a31-colombus.dts | 18 +++ arch/arm/boot/dts/sun6i-a31.dtsi | 61 ++ drivers/i2c/busses/Kconfig | 1 + drivers/i2c/busses/i2c-mv64xxx.c | 31 ++- 5 files changed, 115 insertions(+), 4 deletions(-) -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/5] i2c: mv64xxx: Add support for the Allwinner A31 I2C driver
The Allwinner A31 I2C controller is almost identical to the one used in the
other Allwinner SoCs, except for the fact that it needs to clear the interrupt
by setting the INT_FLAGS bit in the control register, instead of clearing it.
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 9 ++---
drivers/i2c/busses/i2c-mv64xxx.c | 10 ++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index 603003a..2763225 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -4,15 +4,18 @@
Required properties :
- reg : Offset and length of the register set for the device
- - compatible : Should be "marvell,mv64xxx-i2c" or "allwinner,sun4i-i2c"
- or "marvell,mv78230-i2c"
+ - compatible : Should be "marvell,mv64xxx-i2c", "allwinner,sun4i-i2c",
+ "marvell,mv78230-i2c" or "allwinner,sun6i-a31-i2c"
- interrupts : The interrupt number
Optional properties :
- clock-frequency : Desired I2C bus clock frequency in Hz. If not set the
default frequency is 100kHz
- - resets : phandle to the parent reset controller
+
+ - resets : phandle to the parent reset controller. Mandatory
+ whenever you're using the "allwinner,sun6i-a31-i2c"
+ compatible.
Examples:
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 0f6dde5..12ecf4b 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -151,6 +151,7 @@ struct mv64xxx_i2c_data {
/* 5us delay in order to avoid repeated start timing violation */
boolerrata_delay;
struct reset_control*rstc;
+ boolirq_clear_inverted;
};
static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
@@ -568,6 +569,11 @@ mv64xxx_i2c_intr(int irq, void *dev_id)
status = readl(drv_data->reg_base +
drv_data->reg_offsets.status);
mv64xxx_i2c_fsm(drv_data, status);
mv64xxx_i2c_do_action(drv_data);
+
+ if (drv_data->irq_clear_inverted)
+ writel(drv_data->cntl_bits |
MV64XXX_I2C_REG_CONTROL_IFLG,
+ drv_data->reg_base +
drv_data->reg_offsets.control);
+
rc = IRQ_HANDLED;
}
spin_unlock_irqrestore(&drv_data->lock, flags);
@@ -692,6 +698,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = {
*/
static const struct of_device_id mv64xxx_i2c_of_match_table[] = {
{ .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i},
+ { .compatible = "allwinner,sun6i-a31-i2c", .data =
&mv64xxx_i2c_regs_sun4i},
{ .compatible = "marvell,mv64xxx-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{ .compatible = "marvell,mv78230-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{}
@@ -795,6 +802,9 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
drv_data->errata_delay = true;
}
+ if (of_device_is_compatible(np, "allwinner,sun6i-a31-i2c"))
+ drv_data->irq_clear_inverted = true;
+
out:
return rc;
#endif
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/5] ARM: sun6i: colombus: Enable the I2C controllers
The A31 Colombus board has 3 I2C controllers that should be usable. However,
the first one is not working for some reason on the hardware I have been able
to test it on, while it should really be the same controller. Enable the i2c1
and i2c2 busses, and mark i2c0 as in failure in the DT.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun6i-a31-colombus.dts | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-colombus.dts
b/arch/arm/boot/dts/sun6i-a31-colombus.dts
index e5adae3..3898a7b 100644
--- a/arch/arm/boot/dts/sun6i-a31-colombus.dts
+++ b/arch/arm/boot/dts/sun6i-a31-colombus.dts
@@ -28,5 +28,23 @@
pinctrl-0 = <&uart0_pins_a>;
status = "okay";
};
+
+ i2c0: i2c@01c2ac00 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins_a>;
+ status = "fail";
+ };
+
+ i2c1: i2c@01c2b000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c1_pins_a>;
+ status = "okay";
+ };
+
+ i2c2: i2c@01c2b400 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_pins_a>;
+ status = "okay";
+ };
};
};
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/5] i2c: mv64xxx: Add reset deassert call
The Allwinner A31 SoC using that IP has a reset controller maintaining
it reset unless told otherwise.
Add some optional reset support to the driver.
Signed-off-by: Maxime Ripard
---
.../devicetree/bindings/i2c/i2c-mv64xxx.txt | 1 +
drivers/i2c/busses/Kconfig | 1 +
drivers/i2c/busses/i2c-mv64xxx.c| 21 +++--
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
index 82e8f6f..603003a 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt
@@ -12,6 +12,7 @@ Optional properties :
- clock-frequency : Desired I2C bus clock frequency in Hz. If not set the
default frequency is 100kHz
+ - resets : phandle to the parent reset controller
Examples:
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 3b26129..69aa599 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -528,6 +528,7 @@ config I2C_MPC
config I2C_MV64XXX
tristate "Marvell mv64xxx I2C Controller"
depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI)
+ select RESET_CONTROLLER
help
If you say yes to this option, support will be included for the
built-in I2C interface on the Marvell 64xxx line of host bridges.
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 8be7e42..0f6dde5 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -149,6 +150,7 @@ struct mv64xxx_i2c_data {
booloffload_enabled;
/* 5us delay in order to avoid repeated start timing violation */
boolerrata_delay;
+ struct reset_control*rstc;
};
static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
@@ -763,6 +765,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
}
drv_data->irq = irq_of_parse_and_map(np, 0);
+ drv_data->rstc = devm_reset_control_get(dev, NULL);
+ if (IS_ERR(drv_data->rstc)) {
+ if (PTR_ERR(drv_data->rstc) == -EPROBE_DEFER) {
+ rc = -EPROBE_DEFER;
+ goto out;
+ }
+ } else {
+ reset_control_deassert(drv_data->rstc);
+ }
+
/* Its not yet defined how timeouts will be specified in device tree.
* So hard code the value to 1 second.
*/
@@ -845,7 +857,7 @@ mv64xxx_i2c_probe(struct platform_device *pd)
}
if (drv_data->irq < 0) {
rc = -ENXIO;
- goto exit_clk;
+ goto exit_reset;
}
drv_data->adapter.dev.parent = &pd->dev;
@@ -865,7 +877,7 @@ mv64xxx_i2c_probe(struct platform_device *pd)
dev_err(&drv_data->adapter.dev,
"mv64xxx: Can't register intr handler irq%d: %d\n",
drv_data->irq, rc);
- goto exit_clk;
+ goto exit_reset;
} else if ((rc = i2c_add_numbered_adapter(&drv_data->adapter)) != 0) {
dev_err(&drv_data->adapter.dev,
"mv64xxx: Can't add i2c adapter, rc: %d\n", -rc);
@@ -876,6 +888,9 @@ mv64xxx_i2c_probe(struct platform_device *pd)
exit_free_irq:
free_irq(drv_data->irq, drv_data);
+exit_reset:
+ if (pd->dev.of_node && !IS_ERR(drv_data->rstc))
+ reset_control_assert(drv_data->rstc);
exit_clk:
#if defined(CONFIG_HAVE_CLK)
/* Not all platforms have a clk */
@@ -894,6 +909,8 @@ mv64xxx_i2c_remove(struct platform_device *dev)
i2c_del_adapter(&drv_data->adapter);
free_irq(drv_data->irq, drv_data);
+ if (dev->dev.of_node && !IS_ERR(drv_data->rstc))
+ reset_control_assert(drv_data->rstc);
#if defined(CONFIG_HAVE_CLK)
/* Not all platforms have a clk */
if (!IS_ERR(drv_data->clk)) {
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] ARM: sun6i: Enable the I2C controllers
The A31 has 4 I2C controllers that are the same than the one in the
other Allwinner SoCs, except for the fact that they are asserted in
reset by the reset unit.
Add these i2c controllers to the DTSI.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun6i-a31.dtsi | 40
1 file changed, 40 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 5256ad9..7dac496 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -312,6 +312,46 @@
status = "disabled";
};
+ i2c0: i2c@01c2ac00 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2ac00 0x400>;
+ interrupts = <0 6 4>;
+ clocks = <&apb2_gates 0>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 0>;
+ status = "disabled";
+ };
+
+ i2c1: i2c@01c2b000 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2b000 0x400>;
+ interrupts = <0 7 4>;
+ clocks = <&apb2_gates 1>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 1>;
+ status = "disabled";
+ };
+
+ i2c2: i2c@01c2b400 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2b400 0x400>;
+ interrupts = <0 8 4>;
+ clocks = <&apb2_gates 2>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 2>;
+ status = "disabled";
+ };
+
+ i2c3: i2c@01c2b800 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2b800 0x400>;
+ interrupts = <0 9 4>;
+ clocks = <&apb2_gates 3>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 3>;
+ status = "disabled";
+ };
+
gic: interrupt-controller@01c81000 {
compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
reg = <0x01c81000 0x1000>,
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] [PATCH 1/5] i2c: mv64xxx: Add reset deassert call
Hi, On Fri, Jan 10, 2014 at 01:48:41PM +0800, Chen-Yu Tsai wrote: > On Fri, Jan 10, 2014 at 1:19 AM, Maxime Ripard > wrote: > > The Allwinner A31 SoC using that IP has a reset controller maintaining > > it reset unless told otherwise. > > > > Add some optional reset support to the driver. > > > > Signed-off-by: Maxime Ripard > > --- > > Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 1 + > > drivers/i2c/busses/Kconfig| 1 + > > drivers/i2c/busses/i2c-mv64xxx.c | 6 ++ > > 3 files changed, 8 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt > > b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt > > index 82e8f6f..603003a 100644 > > --- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt > > @@ -12,6 +12,7 @@ Optional properties : > > > > - clock-frequency : Desired I2C bus clock frequency in Hz. If not set the > > default frequency is 100kHz > > + - resets : phandle to the parent reset controller > > > > Examples: > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > index 3b26129..69aa599 100644 > > --- a/drivers/i2c/busses/Kconfig > > +++ b/drivers/i2c/busses/Kconfig > > @@ -528,6 +528,7 @@ config I2C_MPC > > config I2C_MV64XXX > > tristate "Marvell mv64xxx I2C Controller" > > depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI) > > + select RESET_CONTROLLER > > help > > If you say yes to this option, support will be included for the > > built-in I2C interface on the Marvell 64xxx line of host bridges. > > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c > > b/drivers/i2c/busses/i2c-mv64xxx.c > > index 8be7e42..d4e2f32 100644 > > --- a/drivers/i2c/busses/i2c-mv64xxx.c > > +++ b/drivers/i2c/busses/i2c-mv64xxx.c > > @@ -17,6 +17,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -743,6 +744,7 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, > > #else > > const struct of_device_id *device; > > struct device_node *np = dev->of_node; > > + struct reset_control *rstc; > > u32 bus_freq, tclk; > > int rc = 0; > > > > @@ -763,6 +765,10 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, > > } > > drv_data->irq = irq_of_parse_and_map(np, 0); > > > > + rstc = devm_reset_control_get(dev, NULL); > > + if (!IS_ERR(rstc)) > > + reset_control_deassert(rstc); > > + > > Do we need to handle -EPROBE_DEFER here? Hmmm, true. > Also no reset_control_assert() in the exit path? > I'm asking because I have similar code in my stmmac patches. That might be a good idea indeed. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[PATCH 4/5] ARM: sun6i: Enable the I2C muxing options
The i2c controllers have a few muxing options on the A31. Enable the
ones found in the A31 Colombus board.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun6i-a31.dtsi | 21 +
1 file changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 7dac496..668de00 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -210,6 +210,27 @@
allwinner,drive = <0>;
allwinner,pull = <0>;
};
+
+ i2c0_pins_a: i2c0@0 {
+ allwinner,pins = "PH14", "PH15";
+ allwinner,function = "i2c0";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c1_pins_a: i2c1@0 {
+ allwinner,pins = "PH16", "PH17";
+ allwinner,function = "i2c1";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c2_pins_a: i2c2@0 {
+ allwinner,pins = "PH18", "PH19";
+ allwinner,function = "i2c2";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
};
ahb1_rst: reset@01c202c0 {
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] ARM: sun6i: colombus: Enable the I2C controllers
The A31 Colombus board has 3 I2C controllers that should be usable. However,
the first one is not working for some reason on the hardware I have been able
to test it on, while it should really be the same controller. Enable the i2c1
and i2c2 busses, and mark i2c0 as in failure in the DT.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun6i-a31-colombus.dts | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-colombus.dts
b/arch/arm/boot/dts/sun6i-a31-colombus.dts
index e5adae3..3898a7b 100644
--- a/arch/arm/boot/dts/sun6i-a31-colombus.dts
+++ b/arch/arm/boot/dts/sun6i-a31-colombus.dts
@@ -28,5 +28,23 @@
pinctrl-0 = <&uart0_pins_a>;
status = "okay";
};
+
+ i2c0: i2c@01c2ac00 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins_a>;
+ status = "fail";
+ };
+
+ i2c1: i2c@01c2b000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c1_pins_a>;
+ status = "okay";
+ };
+
+ i2c2: i2c@01c2b400 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_pins_a>;
+ status = "okay";
+ };
};
};
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] ARM: sun6i: Add support for the A31 I2C controller
Hi everyone, This patchset adds support the A31 i2c controller. This is mostly the same controller as the one found in the other Allwinner SoCs, except for the interrupts acking. On the other SoCs using this driver, the interrupts are acked by clearing the INT_FLAG bit in the control register, while on the A31, the interrupt is acked by writing that bit into the control register. The other difference is that the I2C IP is maintained in reset by a reset controller, so we're adding optionnal support for the reset framework in the driver to deassert the device from reset. Thanks! Maxime Maxime Ripard (5): i2c: mv64xxx: Add reset deassert call i2c: mv64xxx: Add support for the Allwinner A31 I2C driver ARM: sun6i: Enable the I2C controllers ARM: sun6i: Enable the I2C muxing options ARM: sun6i: colombus: Enable the I2C controllers .../devicetree/bindings/i2c/i2c-mv64xxx.txt| 1 + arch/arm/boot/dts/sun6i-a31-colombus.dts | 18 +++ arch/arm/boot/dts/sun6i-a31.dtsi | 61 ++ drivers/i2c/busses/Kconfig | 1 + drivers/i2c/busses/i2c-mv64xxx.c | 16 ++ 5 files changed, 97 insertions(+) -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] i2c: mv64xxx: Add support for the Allwinner A31 I2C driver
The Allwinner A31 I2C controller is almost identical to the one used in the
other Allwinner SoCs, except for the fact that it needs to clear the interrupt
by setting the INT_FLAGS bit in the control register, instead of clearing it.
Signed-off-by: Maxime Ripard
---
drivers/i2c/busses/i2c-mv64xxx.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index d4e2f32..a2f6173 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -150,6 +150,7 @@ struct mv64xxx_i2c_data {
booloffload_enabled;
/* 5us delay in order to avoid repeated start timing violation */
boolerrata_delay;
+ boolirq_clear_inverted;
};
static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
@@ -567,6 +568,11 @@ mv64xxx_i2c_intr(int irq, void *dev_id)
status = readl(drv_data->reg_base +
drv_data->reg_offsets.status);
mv64xxx_i2c_fsm(drv_data, status);
mv64xxx_i2c_do_action(drv_data);
+
+ if (drv_data->irq_clear_inverted)
+ writel(drv_data->cntl_bits |
MV64XXX_I2C_REG_CONTROL_IFLG,
+ drv_data->reg_base +
drv_data->reg_offsets.control);
+
rc = IRQ_HANDLED;
}
spin_unlock_irqrestore(&drv_data->lock, flags);
@@ -691,6 +697,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = {
*/
static const struct of_device_id mv64xxx_i2c_of_match_table[] = {
{ .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i},
+ { .compatible = "allwinner,sun6i-a31-i2c", .data =
&mv64xxx_i2c_regs_sun4i},
{ .compatible = "marvell,mv64xxx-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{ .compatible = "marvell,mv78230-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{}
@@ -789,6 +796,9 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
drv_data->errata_delay = true;
}
+ if (of_device_is_compatible(np, "allwinner,sun6i-a31-i2c"))
+ drv_data->irq_clear_inverted = true;
+
out:
return rc;
#endif
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] i2c: mv64xxx: Add reset deassert call
The Allwinner A31 SoC using that IP has a reset controller maintaining it reset unless told otherwise. Add some optional reset support to the driver. Signed-off-by: Maxime Ripard --- Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt | 1 + drivers/i2c/busses/Kconfig| 1 + drivers/i2c/busses/i2c-mv64xxx.c | 6 ++ 3 files changed, 8 insertions(+) diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt index 82e8f6f..603003a 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt @@ -12,6 +12,7 @@ Optional properties : - clock-frequency : Desired I2C bus clock frequency in Hz. If not set the default frequency is 100kHz + - resets : phandle to the parent reset controller Examples: diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 3b26129..69aa599 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -528,6 +528,7 @@ config I2C_MPC config I2C_MV64XXX tristate "Marvell mv64xxx I2C Controller" depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI) + select RESET_CONTROLLER help If you say yes to this option, support will be included for the built-in I2C interface on the Marvell 64xxx line of host bridges. diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index 8be7e42..d4e2f32 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -743,6 +744,7 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, #else const struct of_device_id *device; struct device_node *np = dev->of_node; + struct reset_control *rstc; u32 bus_freq, tclk; int rc = 0; @@ -763,6 +765,10 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, } drv_data->irq = irq_of_parse_and_map(np, 0); + rstc = devm_reset_control_get(dev, NULL); + if (!IS_ERR(rstc)) + reset_control_deassert(rstc); + /* Its not yet defined how timeouts will be specified in device tree. * So hard code the value to 1 second. */ -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] ARM: sun6i: Enable the I2C controllers
The A31 has 4 I2C controllers that are the same than the one in the
other Allwinner SoCs, except for the fact that they are asserted in
reset by the reset unit.
Add these i2c controllers to the DTSI.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun6i-a31.dtsi | 40
1 file changed, 40 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 5256ad9..7dac496 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -312,6 +312,46 @@
status = "disabled";
};
+ i2c0: i2c@01c2ac00 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2ac00 0x400>;
+ interrupts = <0 6 4>;
+ clocks = <&apb2_gates 0>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 0>;
+ status = "disabled";
+ };
+
+ i2c1: i2c@01c2b000 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2b000 0x400>;
+ interrupts = <0 7 4>;
+ clocks = <&apb2_gates 1>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 1>;
+ status = "disabled";
+ };
+
+ i2c2: i2c@01c2b400 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2b400 0x400>;
+ interrupts = <0 8 4>;
+ clocks = <&apb2_gates 2>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 2>;
+ status = "disabled";
+ };
+
+ i2c3: i2c@01c2b800 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2b800 0x400>;
+ interrupts = <0 9 4>;
+ clocks = <&apb2_gates 3>;
+ clock-frequency = <10>;
+ resets = <&apb2_rst 3>;
+ status = "disabled";
+ };
+
gic: interrupt-controller@01c81000 {
compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
reg = <0x01c81000 0x1000>,
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] i2c-mv64xxx: Add I2C Transaction Generator support
On Tue, Aug 06, 2013 at 02:05:24PM +0200, Gregory CLEMENT wrote: > On 16/07/2013 10:05, Maxime Ripard wrote: > > Hi Gregory, > > > > On Mon, Jul 15, 2013 at 04:24:36PM +0200, Gregory CLEMENT wrote: > >> The I2C Transaction Generator offloads CPU from managing I2C transfer step > >> by step. > >> > >> This feature is currently only available on Armada XP, so usage of this > >> mechanism is activated through device tree. > >> > >> Based on the work of Piotr Ziecik and rewrote to use the new way of > >> handling multiples i2c messages. > >> > >> Signed-off-by: Piotr Ziecik Signed-off-by: Gregory > >> CLEMENT --- > >> drivers/i2c/busses/i2c-mv64xxx.c | 207 > >> --- 1 file changed, 196 insertions(+), > >> 11 deletions(-) > > > > [...] > > > >> + /* + * For controllers embedded in new SoCs activate the + * > >> Transaction Generator support. + */ + if > >> (of_device_is_compatible(np, "marvell,mv78230-i2c")) + > >> drv_data->offload_enabled = true; + > > > > Do you have a reason for not adding it to the match table? I mean, you will > > introduce a new compatible here, but if that compatible is used alone, > > won't probe the driver? That doesn't > > seem very right to me. > > But we shouldn't use it alone: we should always use: > compatible = "marvell,mv78230-i2c", "marvell,mv64xxx-i2c"; > > From my point of view using "marvell,mv78230-i2c" alone is an error. Why is that? If the I2C controller is a new IP with additional features, it should have a full compatible of its own, doesn't it? -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 1/3] i2c-mv64xxx: Add I2C Transaction Generator support
Hi Gregory, On Mon, Jul 15, 2013 at 04:24:36PM +0200, Gregory CLEMENT wrote: > The I2C Transaction Generator offloads CPU from managing I2C > transfer step by step. > > This feature is currently only available on Armada XP, so usage of > this mechanism is activated through device tree. > > Based on the work of Piotr Ziecik and rewrote to use the new way of > handling multiples i2c messages. > > Signed-off-by: Piotr Ziecik > Signed-off-by: Gregory CLEMENT > --- > drivers/i2c/busses/i2c-mv64xxx.c | 207 > --- > 1 file changed, 196 insertions(+), 11 deletions(-) [...] > + /* > + * For controllers embedded in new SoCs activate the > + * Transaction Generator support. > + */ > + if (of_device_is_compatible(np, "marvell,mv78230-i2c")) > + drv_data->offload_enabled = true; > + Do you have a reason for not adding it to the match table? I mean, you will introduce a new compatible here, but if that compatible is used alone, won't probe the driver? That doesn't seem very right to me. Also, you should probably add it to the bindings documentation. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: MTD EEPROM support and driver integration
Hi Mark, On Tue, Jul 09, 2013 at 03:55:10PM +0100, Mark Brown wrote: > On Mon, Jul 08, 2013 at 10:25:38PM +0200, Maxime Ripard wrote: > > On Mon, Jul 08, 2013 at 09:34:26AM +0100, Mark Brown wrote: > > > > I'd really like to see more discussion of this "DT parsing code for > > > regmap" idea... I've missed almost all the context here. > > > The context was that I found we lack a way to simply express the need > > for one driver to get a value from an EEPROM-like device, for example to > > get a MAC Address, or a serial number, in a generic way, without having > > to poke directly with some custom function that would be exported by the > > EEPROM driver. > > This sort of information is often stored in places like flash partitions > too. Are we sure that regmap is a good place to be hooking in here? > The use case is sane, and being able to use regmap to do some of it > seems sensible (I've seen people use OTP in PMICs for similar purposes) > but perhaps an additional layer of abstraction on top makes sense. Ah, I didn't thought it could be stored into a partition. Ok, so using an intermediate abstraction for this makes sense (probably using regmap for all the accesses that are relevant, like i2c, spi or mmio) > > What we've been discussing so far is that: > > - To have a common framework we could base our work on, we could move > > the EEPROM drivers from drivers/misc/eeprom to MTD > > - To declare the ranges that needed to be used by a driver that was > > needing a value from one of those MTD drivers, we would use regmap > > with a MTD backend > > - And since we actually need to declare which ranges and in which > > device one driver would have to retrieve this value from, we were > > actually in need of DT bindings. > > > This is pretty much the only context involved, and we are at the early > > stage of the discussion, so any comment is very welcome :) > > If this stuff is being represented in MTD doesn't MTD already have > adequate abstractions for saying "this region in flash". But otherwise > this seems fine, it's not a generic regmap DT binding but instead rather > more specific than that. Yes, since we seem to be going to a point where regmap will be a convenience in this case, we probably won't need a generic regmap binding, but rather a generic way to define a range and offset into a referenced device. Arnd, the others, is this ok for you? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: MTD EEPROM support and driver integration
On Mon, Jul 08, 2013 at 09:36:14AM +0100, Mark Brown wrote: > On Sun, Jul 07, 2013 at 09:15:01AM +0200, Maxime Ripard wrote: > > On Sat, Jul 06, 2013 at 09:06:49PM +0200, Arnd Bergmann wrote: > > > > We also have a bunch of OTP drivers spread around the kernel, it probably > > > makes sense to consolidate them at the same time, at least on the DT > > > binding > > > side if not the device drivers. > > > From a quick grep, the only one I've seen so far are: > > - imx6q, that has a hook at machine start to poke into its OCOTP to > > retrieve some frequency scaling parameters it seems. I'm not sure > > how the current solution could improve the situation for this > > use-case, but the DT bindings of the OCOTP is just a DT node, with > > no clients, so we have nothing to worry about here. > > - imx28, that has a hook at machine start to look up the MAC address > > values and patch the ethernet controller nodes to add the right > > local-mac-address property. This one could benefit from the new > > bindings, but we already mentionned it, and I intended to develop > > with an imx28 board anyway. > > - picoxcell-pc3x3 DTSI has a node for a OTP device, but they don't > > seem to be doing anything with it, nor do they seem to have a driver > > for it. So I guess we don't care about migrating for this one > > either. > > > Did you have other cases in mind? > > We have some OTP support in the ab8500 and wm831x MFD drivers too but > they just expose the data. I guess you mean ab8100, right? Anyway, thanks for pointing this out Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: MTD EEPROM support and driver integration
On Mon, Jul 08, 2013 at 09:34:26AM +0100, Mark Brown wrote: > On Sat, Jul 06, 2013 at 09:06:49PM +0200, Arnd Bergmann wrote: > > On Saturday 06 July 2013 14:01:12 Maxime Ripard wrote: > > > > > > > a) like interrupts, regs, dmas, clocks, pinctrl, reset, pwm: fixed > > > > > > property names > > > > > > > regmap = <&at25 0xstart 0xlen>; > > > > > > regmap-names = "mac-address"; > > > > > > > b) like gpio, regulator: variable property names > > > > > > > mac-storage = <&at25 0xstart 0xlen>; > > > > > > > It's unfortunate that we already have examples of both. They are > > > > > > largely > > > > > > equivalent, but the tendency is towards the first. > > > > > > I don't have a strong feeling for one against another, so whatever > > > > > works > > > > > best. Both solutions will be a huge improvement anyway > > > > > > Just out of curiosity, is there any advantages besides having a fixed > > > > > property name to the first solution? > > > > > I think it's mostly for consistency: trying to get most subsystems to > > > > do it the same way to make it easier for people to write dts files. > > > > > A lesser point is that it simplifies the driver code if you don't > > > > have to pass a name. > > On the other hand something with human readable names is much more > legible if humans ever have to read or write the DT bindings. This > mostly applies when there are many instances of the property (for > example, many devices have lots of power supplies) or when some > instances of the property are optional (for example, many devices can > use GPIOs for many different functions but usually not all of them are > connected and there's no particular order in which they might get > connected). I guess we would have only a few of these in our cases, so it doesn't really qualify for the "many instances" I guess, but I do find the GPIO/regulator-like properties more intuitive and more readable as well. > > > So that leave us with mainly one path to achieve this goal: > > > - Add a regmap-mtd backend > > > - Add DT parsing code for regmap > > > - Move the EEPROM drivers from misc to mtd > > > Yes, I think that would be good. For the last step, we definitely need > > buy-in from Wolfgand and Jean, as they are maintaining the current eeprom > > drivers. > > I'd really like to see more discussion of this "DT parsing code for > regmap" idea... I've missed almost all the context here. The context was that I found we lack a way to simply express the need for one driver to get a value from an EEPROM-like device, for example to get a MAC Address, or a serial number, in a generic way, without having to poke directly with some custom function that would be exported by the EEPROM driver. And the lack of infrastructure/framework to handle the EEPROM right now only make the thing a bit harder. What we've been discussing so far is that: - To have a common framework we could base our work on, we could move the EEPROM drivers from drivers/misc/eeprom to MTD - To declare the ranges that needed to be used by a driver that was needing a value from one of those MTD drivers, we would use regmap with a MTD backend - And since we actually need to declare which ranges and in which device one driver would have to retrieve this value from, we were actually in need of DT bindings. This is pretty much the only context involved, and we are at the early stage of the discussion, so any comment is very welcome :) Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: MTD EEPROM support and driver integration
On Sat, Jul 06, 2013 at 09:06:49PM +0200, Arnd Bergmann wrote: > On Saturday 06 July 2013 14:01:12 Maxime Ripard wrote: > > What other option would we have? > > > > I also thought about writing an EEPROM framework of its own, but the > > line is really thin between a large EEPROM and say a small SPI > > dataflash, which would make it pretty hard to choose between such a > > framework and MTD. > > Isn't flash by definition block based, while EEPROM can be written > in byte or word units? I think that is a significant difference, although > it doesn't necessarily mean that we can't use MTD for both. Ah, right. > We also have a bunch of OTP drivers spread around the kernel, it probably > makes sense to consolidate them at the same time, at least on the DT binding > side if not the device drivers. From a quick grep, the only one I've seen so far are: - imx6q, that has a hook at machine start to poke into its OCOTP to retrieve some frequency scaling parameters it seems. I'm not sure how the current solution could improve the situation for this use-case, but the DT bindings of the OCOTP is just a DT node, with no clients, so we have nothing to worry about here. - imx28, that has a hook at machine start to look up the MAC address values and patch the ethernet controller nodes to add the right local-mac-address property. This one could benefit from the new bindings, but we already mentionned it, and I intended to develop with an imx28 board anyway. - picoxcell-pc3x3 DTSI has a node for a OTP device, but they don't seem to be doing anything with it, nor do they seem to have a driver for it. So I guess we don't care about migrating for this one either. Did you have other cases in mind? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCHv5 0/8] Add I2C support for Allwinner SoCs
Hi Wolfram, On Sat, Jun 15, 2013 at 01:36:10PM +0200, Wolfram Sang wrote: > Applied to for-next, thanks, especially for respinning the driver! Also > thanks to the testers! Great community work here \o/ Thanks, I applied the other patches to my sunxi/dt-for-3.11 branch. Thanks a lot to Sebastian and Andrew for their patches ! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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: [PATCHv5 0/8] Add I2C support for Allwinner SoCs
Hi Wolfram, On Fri, Jun 14, 2013 at 04:07:20PM +0200, Wolfram Sang wrote: > On Wed, Jun 12, 2013 at 06:53:29PM +0200, Maxime Ripard wrote: > > Hi, > > > > This patchset adds support for the I2C controller found on most of the > > Allwinner SoCs, especially the already supported A10 and A13, and the > > yet to come A31. > > > > This driver leverages the Marvel mv64xxx i2c controller driver, that has > > an almost identical logic, with a slightly different register layout. > > \o/ Yay, that's more like it. Thanks for the rework! > > > It has been tested on a A13-Olinuxino and an A10s-Olinuxino. > > I'd love to get some Tested-by or other tags for Marvell hardware. Any > volunteers? Yes, I'd be more comfortable with Tested-by from Marvell users. I'll probably be able to get my hands on a Marvell board on monday, if no one tested it yet. > > Changes from v4: > > * Don't expose the reg offset structure through the platform data > > Commit message 2/9 still speaks about pdata, but I will fix this locally > if all goes well with this series. > > I will wait a bit more for tags and then pick up patches 1-3. I assume > the rest goes via arm-soc, let me know if I should take them. Yes, I'll take it in my tree and push it to arm-soc afterwards. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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
[PATCHv5 0/8] Add I2C support for Allwinner SoCs
Hi, This patchset adds support for the I2C controller found on most of the Allwinner SoCs, especially the already supported A10 and A13, and the yet to come A31. This driver leverages the Marvel mv64xxx i2c controller driver, that has an almost identical logic, with a slightly different register layout. It has been tested on a A13-Olinuxino and an A10s-Olinuxino. Thanks, Maxime Changes from v4: * Don't expose the reg offset structure through the platform data * Move the register offset structures to the driver and declare them static * Default at marvell's register layout when using platform data, and switch between the Allwinner and the Marvell ones only when using DT. * Remove the pull-ups in the device tree muxings Changes from v3: * Merged the driver in the Marvell mv64xxx i2c controller Changes from v2: * Slightly modified the switch comments again * Removed the of_* calls in favor of platform_get_* functions Changes from v1: * Added comments to the switch statements to clarify when the fall through to the next case is made on purpose * Use devm_ioremap_resource instead of of_iomap * Moved the reset after enabling the clocks * Added Emilio Lopez' patch to add the available i2c controllers to the cubieboard Emilio López (1): ARM: sun4i: cubieboard: Enable the i2c controllers Maxime Ripard (7): i2c: mv64xxx: Add macros to access parts of registers i2c: mv64xxx: make the registers offset configurable i2c: mv64xxx: Add Allwinner sun4i compatible ARM: sunxi: dt: Add i2c controller nodes to the DTSI ARM: sun4i: dt: Add i2c muxing options ARM: sun5i: dt: Add i2c muxing options ARM: sun5i: olinuxino: Enable the i2c controllers arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 12 +++ arch/arm/boot/dts/sun4i-a10.dtsi | 48 arch/arm/boot/dts/sun5i-a13-olinuxino.dts | 18 + arch/arm/boot/dts/sun5i-a13.dtsi | 48 drivers/i2c/busses/Kconfig | 3 +- drivers/i2c/busses/i2c-mv64xxx.c | 118 +++-- 6 files changed, 206 insertions(+), 41 deletions(-) -- 1.8.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
[PATCHv5 6/8] ARM: sun5i: dt: Add i2c muxing options
The i2c controller found on the Allwinner A13 has only one muxing option
available for each controller. Add them to the dtsi
Signed-off-by: Maxime Ripard
Reviewed-by: Tomasz Figa
---
arch/arm/boot/dts/sun5i-a13.dtsi | 21 +
1 file changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 31ebfd7..df96c54 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -184,6 +184,27 @@
allwinner,drive = <0>;
allwinner,pull = <0>;
};
+
+ i2c0_pins_a: i2c0@0 {
+ allwinner,pins = "PB0", "PB1";
+ allwinner,function = "i2c0";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c1_pins_a: i2c1@0 {
+ allwinner,pins = "PB15", "PB16";
+ allwinner,function = "i2c1";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c2_pins_a: i2c2@0 {
+ allwinner,pins = "PB17", "PB18";
+ allwinner,function = "i2c2";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
};
timer@01c20c00 {
--
1.8.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
[PATCHv5 4/8] ARM: sunxi: dt: Add i2c controller nodes to the DTSI
The Allwinner A10 and A13 both have 3 i2c controller embedded.
Add those to the common sunxi dtsi.
Signed-off-by: Maxime Ripard
Reviewed-by: Tomasz Figa
---
arch/arm/boot/dts/sun4i-a10.dtsi | 27 +++
arch/arm/boot/dts/sun5i-a13.dtsi | 27 +++
2 files changed, 54 insertions(+)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 04ff62a..f7e4a96 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -321,5 +321,32 @@
clocks = <&apb1_gates 23>;
status = "disabled";
};
+
+ i2c0: i2c@01c2ac00 {
+ compatible = "allwinner,sun4i-i2c";
+ reg = <0x01c2ac00 0x400>;
+ interrupts = <7>;
+ clocks = <&apb1_gates 0>;
+ clock-frequency = <10>;
+ status = "disabled";
+ };
+
+ i2c1: i2c@01c2b000 {
+ compatible = "allwinner,sun4i-i2c";
+ reg = <0x01c2b000 0x400>;
+ interrupts = <8>;
+ clocks = <&apb1_gates 1>;
+ clock-frequency = <10>;
+ status = "disabled";
+ };
+
+ i2c2: i2c@01c2b400 {
+ compatible = "allwinner,sun4i-i2c";
+ reg = <0x01c2b400 0x400>;
+ interrupts = <9>;
+ clocks = <&apb1_gates 2>;
+ clock-frequency = <10>;
+ status = "disabled";
+ };
};
};
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index f34db19..31ebfd7 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -217,5 +217,32 @@
clocks = <&apb1_gates 19>;
status = "disabled";
};
+
+ i2c0: i2c@01c2ac00 {
+ compatible = "allwinner,sun4i-i2c";
+ reg = <0x01c2ac00 0x400>;
+ interrupts = <7>;
+ clocks = <&apb1_gates 0>;
+ clock-frequency = <10>;
+ status = "disabled";
+ };
+
+ i2c1: i2c@01c2b000 {
+ compatible = "allwinner,sun4i-i2c";
+ reg = <0x01c2b000 0x400>;
+ interrupts = <8>;
+ clocks = <&apb1_gates 1>;
+ clock-frequency = <10>;
+ status = "disabled";
+ };
+
+ i2c2: i2c@01c2b400 {
+ compatible = "allwinner,sun4i-i2c";
+ reg = <0x01c2b400 0x400>;
+ interrupts = <9>;
+ clocks = <&apb1_gates 2>;
+ clock-frequency = <10>;
+ status = "disabled";
+ };
};
};
--
1.8.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
[PATCHv5 2/8] i2c: mv64xxx: make the registers offset configurable
The Allwinner i2c controller uses the same logic as the Marvell one, but
with slightly different register offsets.
Introduce a structure that will be passed by either the pdata or
associated to the compatible strings, and that holds the various
registers that might be needed.
Signed-off-by: Maxime Ripard
---
drivers/i2c/busses/i2c-mv64xxx.c | 101 ---
1 file changed, 62 insertions(+), 39 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index d70a2fda..7ba9bac 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -19,20 +19,12 @@
#include
#include
#include
+#include
#include
#include
#include
#include
-/* Register defines */
-#defineMV64XXX_I2C_REG_SLAVE_ADDR 0x00
-#defineMV64XXX_I2C_REG_DATA0x04
-#defineMV64XXX_I2C_REG_CONTROL 0x08
-#defineMV64XXX_I2C_REG_STATUS 0x0c
-#defineMV64XXX_I2C_REG_BAUD0x0c
-#defineMV64XXX_I2C_REG_EXT_SLAVE_ADDR 0x10
-#defineMV64XXX_I2C_REG_SOFT_RESET 0x1c
-
#define MV64XXX_I2C_ADDR_ADDR(val) ((val & 0x7f) << 1)
#define MV64XXX_I2C_BAUD_DIV_N(val)(val & 0x7)
#define MV64XXX_I2C_BAUD_DIV_M(val)((val & 0xf) << 3)
@@ -89,6 +81,16 @@ enum {
MV64XXX_I2C_ACTION_SEND_STOP,
};
+struct mv64xxx_i2c_regs {
+ u8 addr;
+ u8 ext_addr;
+ u8 data;
+ u8 control;
+ u8 status;
+ u8 clock;
+ u8 soft_reset;
+};
+
struct mv64xxx_i2c_data {
struct i2c_msg *msgs;
int num_msgs;
@@ -98,6 +100,7 @@ struct mv64xxx_i2c_data {
u32 aborting;
u32 cntl_bits;
void __iomem*reg_base;
+ struct mv64xxx_i2c_regs reg_offsets;
u32 addr1;
u32 addr2;
u32 bytes_left;
@@ -116,6 +119,16 @@ struct mv64xxx_i2c_data {
struct i2c_adapter adapter;
};
+static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
+ .addr = 0x00,
+ .ext_addr = 0x10,
+ .data = 0x04,
+ .control= 0x08,
+ .status = 0x0c,
+ .clock = 0x0c,
+ .soft_reset = 0x1c,
+};
+
static void
mv64xxx_i2c_prepare_for_io(struct mv64xxx_i2c_data *drv_data,
struct i2c_msg *msg)
@@ -154,13 +167,13 @@ mv64xxx_i2c_prepare_for_io(struct mv64xxx_i2c_data
*drv_data,
static void
mv64xxx_i2c_hw_init(struct mv64xxx_i2c_data *drv_data)
{
- writel(0, drv_data->reg_base + MV64XXX_I2C_REG_SOFT_RESET);
+ writel(0, drv_data->reg_base + drv_data->reg_offsets.soft_reset);
writel(MV64XXX_I2C_BAUD_DIV_M(drv_data->freq_m) |
MV64XXX_I2C_BAUD_DIV_N(drv_data->freq_n),
- drv_data->reg_base + MV64XXX_I2C_REG_BAUD);
- writel(0, drv_data->reg_base + MV64XXX_I2C_REG_SLAVE_ADDR);
- writel(0, drv_data->reg_base + MV64XXX_I2C_REG_EXT_SLAVE_ADDR);
+ drv_data->reg_base + drv_data->reg_offsets.clock);
+ writel(0, drv_data->reg_base + drv_data->reg_offsets.addr);
+ writel(0, drv_data->reg_base + drv_data->reg_offsets.ext_addr);
writel(MV64XXX_I2C_REG_CONTROL_TWSIEN | MV64XXX_I2C_REG_CONTROL_STOP,
- drv_data->reg_base + MV64XXX_I2C_REG_CONTROL);
+ drv_data->reg_base + drv_data->reg_offsets.control);
drv_data->state = MV64XXX_I2C_STATE_IDLE;
}
@@ -282,7 +295,7 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
drv_data->cntl_bits |= MV64XXX_I2C_REG_CONTROL_START;
writel(drv_data->cntl_bits,
- drv_data->reg_base + MV64XXX_I2C_REG_CONTROL);
+ drv_data->reg_base + drv_data->reg_offsets.control);
drv_data->msgs++;
drv_data->num_msgs--;
@@ -300,48 +313,48 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
case MV64XXX_I2C_ACTION_CONTINUE:
writel(drv_data->cntl_bits,
- drv_data->reg_base + MV64XXX_I2C_REG_CONTROL);
+ drv_data->reg_base + drv_data->reg_offsets.control);
break;
case MV64XXX_I2C_ACTION_SEND_START:
writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START,
- drv_data->reg_base + MV64XXX_I2C_REG_CONTROL);
+ drv_data->reg_base + drv_data->reg_offsets.control);
break;
case MV64XXX_I2C_ACTION_SEND_ADDR_1:
w
[PATCHv5 1/8] i2c: mv64xxx: Add macros to access parts of registers
These macros make it more comprehensive to access to useful masked and
shifted area of the various registers used.
Signed-off-by: Maxime Ripard
---
drivers/i2c/busses/i2c-mv64xxx.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 6356439..d70a2fda 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -33,6 +33,10 @@
#defineMV64XXX_I2C_REG_EXT_SLAVE_ADDR 0x10
#defineMV64XXX_I2C_REG_SOFT_RESET 0x1c
+#define MV64XXX_I2C_ADDR_ADDR(val) ((val & 0x7f) << 1)
+#define MV64XXX_I2C_BAUD_DIV_N(val)(val & 0x7)
+#define MV64XXX_I2C_BAUD_DIV_M(val)((val & 0xf) << 3)
+
#defineMV64XXX_I2C_REG_CONTROL_ACK 0x0004
#defineMV64XXX_I2C_REG_CONTROL_IFLG0x0008
#defineMV64XXX_I2C_REG_CONTROL_STOP0x0010
@@ -133,7 +137,7 @@ mv64xxx_i2c_prepare_for_io(struct mv64xxx_i2c_data
*drv_data,
drv_data->addr1 = 0xf0 | (((u32)msg->addr & 0x300) >> 7) | dir;
drv_data->addr2 = (u32)msg->addr & 0xff;
} else {
- drv_data->addr1 = ((u32)msg->addr & 0x7f) << 1 | dir;
+ drv_data->addr1 = MV64XXX_I2C_ADDR_ADDR((u32)msg->addr) | dir;
drv_data->addr2 = 0;
}
}
@@ -151,7 +155,7 @@ static void
mv64xxx_i2c_hw_init(struct mv64xxx_i2c_data *drv_data)
{
writel(0, drv_data->reg_base + MV64XXX_I2C_REG_SOFT_RESET);
- writeldrv_data->freq_m & 0xf) << 3) | (drv_data->freq_n & 0x7)),
+ writel(MV64XXX_I2C_BAUD_DIV_M(drv_data->freq_m) |
MV64XXX_I2C_BAUD_DIV_N(drv_data->freq_n),
drv_data->reg_base + MV64XXX_I2C_REG_BAUD);
writel(0, drv_data->reg_base + MV64XXX_I2C_REG_SLAVE_ADDR);
writel(0, drv_data->reg_base + MV64XXX_I2C_REG_EXT_SLAVE_ADDR);
--
1.8.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
[PATCHv5 5/8] ARM: sun4i: dt: Add i2c muxing options
The i2c controller found on the Allwinner A10 has only one muxing option
available for each controller. Add them to the dtsi
Signed-off-by: Maxime Ripard
Reviewed-by: Tomasz Figa
---
arch/arm/boot/dts/sun4i-a10.dtsi | 21 +
1 file changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index f7e4a96..9e6fb45 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -228,6 +228,27 @@
allwinner,drive = <0>;
allwinner,pull = <0>;
};
+
+ i2c0_pins_a: i2c0@0 {
+ allwinner,pins = "PB0", "PB1";
+ allwinner,function = "i2c0";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c1_pins_a: i2c1@0 {
+ allwinner,pins = "PB18", "PB19";
+ allwinner,function = "i2c1";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ i2c2_pins_a: i2c2@0 {
+ allwinner,pins = "PB20", "PB21";
+ allwinner,function = "i2c2";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
};
timer@01c20c00 {
--
1.8.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
[PATCHv5 8/8] ARM: sun4i: cubieboard: Enable the i2c controllers
From: Emilio López
The Cubieboard makes use of the first two i2c controllers found on the
Allwinner A10; i2c-0 is used internally for the PMIC, while i2c-1
is exposed on the board headers. This patch enables them in the device
tree.
Signed-off-by: Emilio López
Signed-off-by: Maxime Ripard
Reviewed-by: Tomasz Figa
---
arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
index e752b57..757c4cd 100644
--- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
+++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
@@ -56,6 +56,18 @@
pinctrl-0 = <&uart0_pins_a>;
status = "okay";
};
+
+ i2c0: i2c@01c2ac00 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins_a>;
+ status = "okay";
+ };
+
+ i2c1: i2c@01c2b000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c1_pins_a>;
+ status = "okay";
+ };
};
leds {
--
1.8.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
[PATCHv5 3/8] i2c: mv64xxx: Add Allwinner sun4i compatible
Add the compatible string for the Allwinner A10 i2c controller and the
associated register layout.
Signed-off-by: Maxime Ripard
---
drivers/i2c/busses/Kconfig | 3 ++-
drivers/i2c/busses/i2c-mv64xxx.c | 11 +++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 631736e..5dc4148 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -507,10 +507,11 @@ config I2C_MPC
config I2C_MV64XXX
tristate "Marvell mv64xxx I2C Controller"
- depends on (MV64X60 || PLAT_ORION)
+ depends on (MV64X60 || PLAT_ORION || ARCH_SUNXI)
help
If you say yes to this option, support will be included for the
built-in I2C interface on the Marvell 64xxx line of host bridges.
+ This driver is also used for Allwinner SoCs I2C controllers.
This driver can also be built as a module. If so, the module
will be called i2c-mv64xxx.
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 7ba9bac..7a0e39b 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -129,6 +129,16 @@ static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
.soft_reset = 0x1c,
};
+static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_sun4i = {
+ .addr = 0x00,
+ .ext_addr = 0x04,
+ .data = 0x08,
+ .control= 0x0c,
+ .status = 0x10,
+ .clock = 0x14,
+ .soft_reset = 0x18,
+};
+
static void
mv64xxx_i2c_prepare_for_io(struct mv64xxx_i2c_data *drv_data,
struct i2c_msg *msg)
@@ -509,6 +519,7 @@ static const struct i2c_algorithm mv64xxx_i2c_algo = {
*
*/
static const struct of_device_id mv64xxx_i2c_of_match_table[] = {
+ { .compatible = "allwinner,sun4i-i2c", .data = &mv64xxx_i2c_regs_sun4i},
{ .compatible = "marvell,mv64xxx-i2c", .data =
&mv64xxx_i2c_regs_mv64xxx},
{}
};
--
1.8.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
[PATCHv5 7/8] ARM: sun5i: olinuxino: Enable the i2c controllers
The A13-Olinuxino makes use of the 3 i2c controllers found on the Allwinner
A13. Enable them in the device tree.
Signed-off-by: Maxime Ripard
Reviewed-by: Tomasz Figa
---
arch/arm/boot/dts/sun5i-a13-olinuxino.dts | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
index 3ca5506..80497e3 100644
--- a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
+++ b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
@@ -37,6 +37,24 @@
pinctrl-0 = <&uart1_pins_b>;
status = "okay";
};
+
+ i2c0: i2c@01c2ac00 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins_a>;
+ status = "okay";
+ };
+
+ i2c1: i2c@01c2b000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c1_pins_a>;
+ status = "okay";
+ };
+
+ i2c2: i2c@01c2b400 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_pins_a>;
+ status = "okay";
+ };
};
leds {
--
1.8.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: [PATCHv4 2/9] i2c: mv64xxx: make the registers offset configurable
Hi Sebastian,
On Wed, Jun 12, 2013 at 05:03:12PM +0200, Sebastian Hesselbarth wrote:
> On 06/12/13 16:44, Maxime Ripard wrote:
> >Hi Russel,
> >
> >On Wed, Jun 12, 2013 at 02:57:35PM +0100, Russell King - ARM Linux wrote:
> >>On Wed, Jun 12, 2013 at 10:07:11AM +0200, Maxime Ripard wrote:
> >>>The Allwinner i2c controller uses the same logic as the Marvell one, but
> >>>with slightly different register offsets.
> >>>
> >>>Introduce a structure that will be passed by either the pdata or
> >>>associated to the compatible strings, and that holds the various
> >>>registers that might be needed.
> >>
> >>I don't like this change. It introduces further indirection where it's
> >>not really necessary, and it's also using platform data to specify this
> >>which is in the opposite direction to what's required for moving towards
> >>DT.
> >
> >Well, some users of this aren't converted to DT, hence why I made the
> >changes to the platform_data.
>
> Actually, this is not quite true. Yes of course, there are still users
> of non-DT Marvell SoCs and it is still in the progress of full-DT. But
> also ppc is using DT, except that they parse it and put in in
> platform_data. Reasonable since back then, there was no global DT API
> available.
Ah, I see, thanks for the insight. I was here referring more
specifically to Orion that seems to be still stuck with !DT at the
moment, at least partially.
> IMHO for the time in between (i.e. now) check for pdev->dev.of_node
> and !pdev->dev.platform_data will allow you to distinguish all users
> perfectly:
>
> - non-DT has platform_data set only
> - ppc DT has of_node and platform_data set
> - pure DT has of_node set only
>
> This will allow you to limit your register offset modifications to
> Allwinner exclusively and for pure DT (if that is what you want for
> Allwinner).
>
> Checkout mv643xx_eth in net-next where the above discrimination
> strategy was chosen.
>
> [...]
> >>I'd suggest making the default register offsets be the drivers existing
> >>offsets, and allowing it to be overriden. That nicely sorts out the
> >>next comment below, and also gets rid of it in platform data. Moreover,
> >>if you're going to re-use this driver, you should do it via a different
> >>"compatible" name in DT, which the driver can then use to identify the
> >>different register set layout.
> >
> >The logic here will change quite a bit in the next iteration thanks to
> >the comments I received.
> >
> >I'm now using a platform_device_id structure to match the name of the
> >driver just like what was done with the DT in that patchset. This also
> >removes the need to add the regs field to the platform data and ...
>
> Also here, if Allwinner is pure DT, you can call some
> mv643xx_i2c_of_probe() for pure DT only with the above discrimination.
Unless I'm missing something, isn't it what's already in place here?
We have:
if (pdata) {
/* Fill in the driver data structure from pdata */
} else if (pd->dev.of_node) {
/* Fill in the driver data structure from dt */
} else {
return -EFAIL;
}
I guess that should cover all the cases you mentionned, even the PPC
one, right?
Now, the question about what content do we find in these platform_data
is actually a different one. This patch passed the regs offset as a
member of those. We all agreed that it was not the most elegant solution
(and like you mentionned, I will never use this pdata structure anyway
for the Allwinner stuff).
I guess we could just take the marvell offsets when using pdata, and use
different register offsets based on the compatibles when loading from
dt?
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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: [PATCHv4 2/9] i2c: mv64xxx: make the registers offset configurable
On Wed, Jun 12, 2013 at 03:51:39PM +0100, Russell King - ARM Linux wrote:
> On Wed, Jun 12, 2013 at 04:44:47PM +0200, Maxime Ripard wrote:
> > Hi Russel,
> >
> > On Wed, Jun 12, 2013 at 02:57:35PM +0100, Russell King - ARM Linux wrote:
> > > It'd be much better to copy the offsets themselves in drv_data. You're
> > > only talking about 7 bytes here, so there's no worry about bloating the
> > > drv_data structure.
> >
> > It was more about keeping things separated. Moreover, the probe
> > function gets smaller, since you have only a pointer to pass on, instead
> > of assigning those 7 bytes.
>
> struct driver_data {
> struct mv64xxx_i2c_regs reg_offsets;
> };
>
> struct driver_data *drv_data;
>
> memcpy(drv_data->reg_offsets, reg_offsets,
> sizeof(drv_data->reg_offsets));
>
> No need to write it each member as a separate assignment.
Ah right. I previously understood that you wanted a single variable for
each register in the driver data structure.
I'll do like you suggested.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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: [PATCHv4 2/9] i2c: mv64xxx: make the registers offset configurable
Hi Russel,
On Wed, Jun 12, 2013 at 02:57:35PM +0100, Russell King - ARM Linux wrote:
> On Wed, Jun 12, 2013 at 10:07:11AM +0200, Maxime Ripard wrote:
> > The Allwinner i2c controller uses the same logic as the Marvell one, but
> > with slightly different register offsets.
> >
> > Introduce a structure that will be passed by either the pdata or
> > associated to the compatible strings, and that holds the various
> > registers that might be needed.
>
> I don't like this change. It introduces further indirection where it's
> not really necessary, and it's also using platform data to specify this
> which is in the opposite direction to what's required for moving towards
> DT.
Well, some users of this aren't converted to DT, hence why I made the
changes to the platform_data.
> > @@ -154,13 +147,13 @@ mv64xxx_i2c_prepare_for_io(struct mv64xxx_i2c_data
> > *drv_data,
> > static void
> > mv64xxx_i2c_hw_init(struct mv64xxx_i2c_data *drv_data)
> > {
> > - writel(0, drv_data->reg_base + MV64XXX_I2C_REG_SOFT_RESET);
> > + writel(0, drv_data->reg_base + drv_data->regs->soft_reset);
>
> It'd be much better to copy the offsets themselves in drv_data. You're
> only talking about 7 bytes here, so there's no worry about bloating the
> drv_data structure.
It was more about keeping things separated. Moreover, the probe
function gets smaller, since you have only a pointer to pass on, instead
of assigning those 7 bytes.
And since Gregory Clement is working on adding more registers, I believe
it makes more sense to have things separate.
>
> > @@ -611,8 +619,9 @@ mv64xxx_i2c_probe(struct platform_device *pd)
> > drv_data->freq_n = pdata->freq_n;
> > drv_data->irq = platform_get_irq(pd, 0);
> > drv_data->adapter.timeout = msecs_to_jiffies(pdata->timeout);
> > + drv_data->regs = pdata->regs;
> > } else if (pd->dev.of_node) {
> > - rc = mv64xxx_of_config(drv_data, pd->dev.of_node);
> > + rc = mv64xxx_of_config(drv_data, &pd->dev);
>
> I'd suggest making the default register offsets be the drivers existing
> offsets, and allowing it to be overriden. That nicely sorts out the
> next comment below, and also gets rid of it in platform data. Moreover,
> if you're going to re-use this driver, you should do it via a different
> "compatible" name in DT, which the driver can then use to identify the
> different register set layout.
The logic here will change quite a bit in the next iteration thanks to
the comments I received.
I'm now using a platform_device_id structure to match the name of the
driver just like what was done with the DT in that patchset. This also
removes the need to add the regs field to the platform data and ...
>
> > +struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
> > + .addr = 0x00,
> > + .ext_addr = 0x10,
> > + .data = 0x04,
> > + .control = 0x08,
> > + .status = 0x0c,
> > + .clock = 0x0c,
> > + .soft_reset = 0x1c,
> > +};
>
> No, this means every file which includes this header ends up defining
> this structure, which is globally visiable, and therefore its a recipe
> for link failures.
solves this as well.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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: [PATCHv4 0/9] Add I2C support for Allwinner SoCs
On Wed, Jun 12, 2013 at 02:38:12PM +0200, Arnd Bergmann wrote: > On Wednesday 12 June 2013, Maxime Ripard wrote: > > The Marvell and Allwinner controllers share the exact same logic (which > > is definitely not trivial), based on a finite state machine that > > triggers interrupts at each change of state, each state being a state in > > the I2C protocol (like address sent, data received with an ACK, etc.). > > > > The weird thing is that the only difference between the two controllers > > is the register offsets, and that's it. The state numbers, bit index, > > etc, are exactly the same. > > Ok, cool. Great someone noticed! Kudos to Wolfram :) > > So yes, I think they both licensed the same IP. > > I wonder if it's the Mentor Graphics Inventra mi2c block, which > would make sense given that Allwinner also uses musb. The only datasheet or manual I have been able to find is http://www.mentor.com/products/ip/peripheral/ip_interface/upload/mi2c_pd.pdf which doesn't give a lot of details. Yet, from what is shown and explained in the second page, it looks like it could be this IP. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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
